Merge branch 'master' into AGDNS-2556-custom-update-url

CHANGELOG.md

@@ -38,6 +38,12 @@ NOTE: Add new changes BELOW THIS COMMENT.
   permissions for the security-sensitive files and directories, which caused
   issues on Windows ([#7400]).
 
+### Fixed
+
+- Goroutine leak during configuration update resulting in increased response
+  time. ([#6818]).
+
+[#6818]: https://github.com/AdguardTeam/AdGuardHome/issues/6818
 [#7400]: https://github.com/AdguardTeam/AdGuardHome/issues/7400
 
 [go-1.23.3]: https://groups.google.com/g/golang-announce/c/X5KodEJYuqI

internal/dnsforward/dnsforward.go

@@ -818,6 +818,8 @@ func (s *Server) proxy() (p *proxy.Proxy) {
 }
 
 // Reconfigure applies the new configuration to the DNS server.
+//
+// TODO(a.garipov): This whole piece of API is weird and needs to be remade.
 func (s *Server) Reconfigure(conf *ServerConfig) error {
 	s.serverLock.Lock()
 	defer s.serverLock.Unlock()
@@ -831,14 +833,15 @@ func (s *Server) Reconfigure(conf *ServerConfig) error {
 	// We wait for some time and hope that this fd will be closed.
 	time.Sleep(100 * time.Millisecond)
 
-	// TODO(a.garipov): This whole piece of API is weird and needs to be remade.
+	if s.addrProc != nil {
+		err := s.addrProc.Close()
+		if err != nil {
+			log.Error("dnsforward: closing address processor: %s", err)
+		}
+	}
+
 	if conf == nil {
 		conf = &s.conf
-	} else {
-		closeErr := s.addrProc.Close()
-		if closeErr != nil {
-			log.Error("dnsforward: closing address processor: %s", closeErr)
-		}
 	}
 
 	// TODO(e.burkov):  It seems an error here brings the server down, which is

internal/dnsforward/dnsforward_test.go

@@ -500,6 +500,10 @@ func TestServerRace(t *testing.T) {
 }
 
 func TestSafeSearch(t *testing.T) {
+	const (
+		googleSafeSearch = "forcesafesearch.google.com."
+	)
+
 	safeSearchConf := filtering.SafeSearchConfig{
 		Enabled: true,
 		Google:  true,
@@ -536,10 +540,17 @@ func TestSafeSearch(t *testing.T) {
 		ServePlainDNS: true,
 	}
 	s := createTestServer(t, filterConf, forwardConf)
-	startDeferStop(t, s)
 
+	ups := aghtest.NewUpstreamMock(func(req *dns.Msg) (resp *dns.Msg, err error) {
+		pt := testutil.PanicT{}
+		assert.Equal(pt, googleSafeSearch, req.Question[0].Name)
+
+		return aghtest.MatchedResponse(req, dns.TypeA, googleSafeSearch, "1.2.3.4"), nil
+	})
+	s.conf.UpstreamConfig.Upstreams = []upstream.Upstream{ups}
+
+	startDeferStop(t, s)
 	addr := s.dnsProxy.Addr(proxy.ProtoUDP).String()
-	client := &dns.Client{}
 
 	yandexIP := netip.AddrFrom4([4]byte{213, 180, 193, 56})
 
@@ -585,17 +596,9 @@ func TestSafeSearch(t *testing.T) {
 		t.Run(tc.host, func(t *testing.T) {
 			req := createTestMessage(tc.host)
 
-			// TODO(a.garipov):  Create our own helper for this.
 			var reply *dns.Msg
-			once := &sync.Once{}
-			require.EventuallyWithT(t, func(c *assert.CollectT) {
-				r, _, errExch := client.Exchange(req, addr)
-				if assert.NoError(c, errExch) {
-					once.Do(func() { reply = r })
-				} else {
-					t.Logf("got error: %v", errExch)
-				}
-			}, testTimeout*10, testTimeout)
+			reply, err = dns.Exchange(req, addr)
+			require.NoError(t, err)
 
 			if tc.wantCNAME != "" {
 				require.Len(t, reply.Answer, 2)