From cd09ba63b62f78214e8c3b2b82b6a1ba1eff0bc4 Mon Sep 17 00:00:00 2001
From: Stanislav Chzhen <s.chzhen@adguard.com>
Date: Fri, 20 Oct 2023 15:45:57 +0300
Subject: [PATCH] Pull request 2048: AG-26594-fix-filtering-race

Squashed commit of the following:

commit 9b5b035aa3edfe20cbc26772b8a5c76d81288116
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Oct 20 13:00:29 2023 +0300

    filtering: imp code

commit 406f4015d80d8b11fbd0aeacfabe686931bbe3fb
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Oct 19 15:04:13 2023 +0300

    filtering: fix race
---
 internal/filtering/filter.go    | 24 ----------
 internal/filtering/filtering.go | 81 ++++++++++++++++++++++++---------
 2 files changed, 60 insertions(+), 45 deletions(-)

diff --git a/internal/filtering/filter.go b/internal/filtering/filter.go
index 169b2d51..64befc73 100644
--- a/internal/filtering/filter.go
+++ b/internal/filtering/filter.go
@@ -263,30 +263,6 @@ func assignUniqueFilterID() int64 {
 	return value
 }
 
-// Sets up a timer that will be checking for filters updates periodically
-func (d *DNSFilter) periodicallyRefreshFilters() {
-	const maxInterval = 1 * 60 * 60
-	ivl := 5 // use a dynamically increasing time interval
-	for {
-		isNetErr, ok := false, false
-		if d.conf.FiltersUpdateIntervalHours != 0 {
-			_, isNetErr, ok = d.tryRefreshFilters(true, true, false)
-			if ok && !isNetErr {
-				ivl = maxInterval
-			}
-		}
-
-		if isNetErr {
-			ivl *= 2
-			if ivl > maxInterval {
-				ivl = maxInterval
-			}
-		}
-
-		time.Sleep(time.Duration(ivl) * time.Second)
-	}
-}
-
 // tryRefreshFilters is like [refreshFilters], but backs down if the update is
 // already going on.
 //
diff --git a/internal/filtering/filtering.go b/internal/filtering/filtering.go
index c9f52dfc..6fae9c60 100644
--- a/internal/filtering/filtering.go
+++ b/internal/filtering/filtering.go
@@ -257,6 +257,9 @@ type DNSFilter struct {
 	// conf contains filtering parameters.
 	conf *Config
 
+	// done is the channel to signal to stop running filters updates loop.
+	done chan struct{}
+
 	// Channel for passing data to filters-initializer goroutine
 	filtersInitializerChan chan filtersInitializerParams
 	filtersInitializerLock sync.Mutex
@@ -424,24 +427,15 @@ func (d *DNSFilter) setFilters(blockFilters, allowFilters []Filter, async bool)
 	return d.initFiltering(allowFilters, blockFilters)
 }
 
-// Starts initializing new filters by signal from channel
-func (d *DNSFilter) filtersInitializer() {
-	for {
-		params := <-d.filtersInitializerChan
-		err := d.initFiltering(params.allowFilters, params.blockFilters)
-		if err != nil {
-			log.Error("filtering: initializing: %s", err)
-
-			continue
-		}
-	}
-}
-
 // Close - close the object
 func (d *DNSFilter) Close() {
 	d.engineLock.Lock()
 	defer d.engineLock.Unlock()
 
+	if d.done != nil {
+		d.done <- struct{}{}
+	}
+
 	d.reset()
 }
 
@@ -1131,19 +1125,64 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
 	return d, nil
 }
 
-// Start - start the module:
-// . start async filtering initializer goroutine
-// . register web handlers
+// Start registers web handlers and starts filters updates loop.
 func (d *DNSFilter) Start() {
 	d.filtersInitializerChan = make(chan filtersInitializerParams, 1)
-	go d.filtersInitializer()
+	d.done = make(chan struct{}, 1)
 
 	d.RegisterFilteringHandlers()
 
-	// Here we should start updating filters,
-	//  but currently we can't wake up the periodic task to do so.
-	// So for now we just start this periodic task from here.
-	go d.periodicallyRefreshFilters()
+	go d.updatesLoop()
+}
+
+// updatesLoop initializes new filters and checks for filters updates in a loop.
+func (d *DNSFilter) updatesLoop() {
+	defer log.OnPanic("filtering: updates loop")
+
+	ivl := time.Second * 5
+	t := time.NewTimer(ivl)
+
+	for {
+		select {
+		case params := <-d.filtersInitializerChan:
+			err := d.initFiltering(params.allowFilters, params.blockFilters)
+			if err != nil {
+				log.Error("filtering: initializing: %s", err)
+
+				continue
+			}
+		case <-t.C:
+			ivl = d.periodicallyRefreshFilters(ivl)
+			t.Reset(ivl)
+		case <-d.done:
+			t.Stop()
+
+			return
+		}
+	}
+}
+
+// periodicallyRefreshFilters checks for filters updates and returns time
+// interval for the next update.
+func (d *DNSFilter) periodicallyRefreshFilters(ivl time.Duration) (nextIvl time.Duration) {
+	const maxInterval = time.Hour
+
+	if d.conf.FiltersUpdateIntervalHours == 0 {
+		return ivl
+	}
+
+	isNetErr, ok := false, false
+	_, isNetErr, ok = d.tryRefreshFilters(true, true, false)
+
+	if ok && !isNetErr {
+		ivl = maxInterval
+	} else if isNetErr {
+		ivl *= 2
+		// TODO(s.chzhen):  Use built-in function max in Go 1.21.
+		ivl = mathutil.Max(ivl, maxInterval)
+	}
+
+	return ivl
 }
 
 // Safe browsing and parental control methods.