From da45eabc313cc4f49c02fd14c706ac91f9a6955c Mon Sep 17 00:00:00 2001
From: Eugene Burkov <e.burkov@adguard.com>
Date: Wed, 6 Oct 2021 13:14:41 +0300
Subject: [PATCH] Pull request: 3655 shutdown panic

Merge in DNS/adguard-home from 3655-stop-panic to master

Updates #3655.

Squashed commit of the following:

commit 5ffe5193d79a82c70e3f9f547ba52ca20f7abdeb
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Oct 6 13:06:33 2021 +0300

    dnsforward: imp code, docs

commit 3a4f04f50cd8e0d59edb9e3824f1d55bab9c73a6
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Oct 5 16:42:25 2021 +0300

    dnsforward: lock to read proxy
---
 CHANGELOG.md                      |  2 ++
 internal/dnsforward/dns.go        | 12 ++++++++++--
 internal/dnsforward/dnsforward.go | 28 +++++++++++++++++-----------
 internal/dnsforward/msg.go        | 12 ++++++++++--
 4 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d1062462..7b017cf0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -114,6 +114,7 @@ In this release, the schema version has changed from 10 to 12.
 
 ### Fixed
 
+- Occasional panic during 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
 
 
 
diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go
index fc2f245a..4f1b8d3a 100644
--- a/internal/dnsforward/dns.go
+++ b/internal/dnsforward/dns.go
@@ -540,8 +540,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 = srvClosedErr
+
+		return resultCodeError
+	}
+
+	if ctx.err = prx.Resolve(d); ctx.err != nil {
 		return resultCodeError
 	}
 
diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go
index 80d51548..a62a6614 100644
--- a/internal/dnsforward/dnsforward.go
+++ b/internal/dnsforward/dnsforward.go
@@ -551,6 +551,21 @@ func (s *Server) IsRunning() bool {
 	return s.isRunning
 }
 
+// srvClosedErr is returned when the method can't complete without unacessible
+// data from the closing server.
+const srvClosedErr errors.Error = "server is closed"
+
+// proxy returns a pointer to the current DNS proxy instance.  If p is nil, the
+// server is closing.
+//
+// See https://github.com/AdguardTeam/AdGuardHome/issues/3655.
+func (s *Server) proxy() (p *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 +596,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)
 	}
 }
 
diff --git a/internal/dnsforward/msg.go b/internal/dnsforward/msg.go
index 3735d71c..c5333ba2 100644
--- a/internal/dnsforward/msg.go
+++ b/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: %s", srvClosedErr)
+
+		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)
 	}