dnsforward: lock to read proxy

CHANGELOG.md

@@ -114,6 +114,7 @@ In this release, the schema version has changed from 10 to 12.
 
 ### Fixed
 
+- Panic while shutdown ([#3655]).
 - Addition of IPs into only one as opposed to all matching ipsets on Linux
   ([#3638]).
 - Removal of temporary filter files ([#3567]).
@@ -202,6 +203,7 @@ In this release, the schema version has changed from 10 to 12.
 [#3579]: https://github.com/AdguardTeam/AdGuardHome/issues/3579
 [#3607]: https://github.com/AdguardTeam/AdGuardHome/issues/3607
 [#3638]: https://github.com/AdguardTeam/AdGuardHome/issues/3638
+[#3655]: https://github.com/AdguardTeam/AdGuardHome/issues/3655
 
 
 

internal/dnsforward/dns.go

@@ -8,6 +8,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/dnsproxy/proxy"
+	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/stringutil"
@@ -540,8 +541,16 @@ func (s *Server) processUpstream(ctx *dnsContext) (rc resultCode) {
 		}
 	}
 
-	// request was not filtered so let it be processed further
-	if ctx.err = s.dnsProxy.Resolve(d); ctx.err != nil {
+	// Process the request further since it wasn't filtered.
+
+	prx := s.proxy()
+	if prx == nil {
+		ctx.err = errors.Error("server is closed")
+
+		return resultCodeError
+	}
+
+	if ctx.err = prx.Resolve(d); ctx.err != nil {
 		return resultCodeError
 	}
 

internal/dnsforward/dnsforward.go

@@ -551,6 +551,19 @@ func (s *Server) IsRunning() bool {
 	return s.isRunning
 }
 
+// proxy returns the underlying *proxy.Proxy from s to be accessed in
+// concurrent-safe methods.  It should prevent problems caused by unexpected
+// rewrites of the field inside s.  It returns nil when s is stopping or already
+// closed itself.
+//
+// See https://github.com/AdguardTeam/AdGuardHome/issues/3655.
+func (s *Server) proxy() (prx *proxy.Proxy) {
+	s.serverLock.RLock()
+	defer s.serverLock.RUnlock()
+
+	return s.dnsProxy
+}
+
 // Reconfigure applies the new configuration to the DNS server.
 func (s *Server) Reconfigure(config *ServerConfig) error {
 	s.serverLock.Lock()
@@ -581,17 +594,8 @@ func (s *Server) Reconfigure(config *ServerConfig) error {
 
 // ServeHTTP is a HTTP handler method we use to provide DNS-over-HTTPS.
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	var p *proxy.Proxy
-
-	func() {
-		s.serverLock.RLock()
-		defer s.serverLock.RUnlock()
-
-		p = s.dnsProxy
-	}()
-
-	if p != nil {
-		p.ServeHTTP(w, r)
+	if prx := s.proxy(); prx != nil {
+		prx.ServeHTTP(w, r)
 	}
 }
 

internal/dnsforward/msg.go

@@ -249,9 +249,17 @@ func (s *Server) genBlockedHost(request *dns.Msg, newAddr string, d *proxy.DNSCo
 		Req:       &replReq,
 	}
 
-	err := s.dnsProxy.Resolve(newContext)
+	prx := s.proxy()
+	if prx == nil {
+		log.Debug("dns server is closed")
+
+		return s.genServerFailure(request)
+	}
+
+	err := prx.Resolve(newContext)
 	if err != nil {
-		log.Printf("Couldn't look up replacement host %q: %s", newAddr, err)
+		log.Printf("couldn't look up replacement host %q: %s", newAddr, err)
+
 		return s.genServerFailure(request)
 	}