From bb7d68dedade76fe56b25978d7e99c40c06f6747 Mon Sep 17 00:00:00 2001
From: stasatdaglabs <39559713+stasatdaglabs@users.noreply.github.com>
Date: Mon, 2 Dec 2019 18:08:32 +0200
Subject: [PATCH] [NOD-484] Fix deadlock between p2p server and sync manager
 during shutdown (#508)

* [NOD-484] Fix deadlock between p2p server and sync manager during shutdown.

* [NOD-484] Fix quitWaitGroup.Wait() potentially not waiting in some scenarios.

* [NOD-484] Add a comment explaining quitWaitGroup.

* [NOD-484] Fix typo.

* [NOD-484] Add etc to comment.
---
 server/p2p/p2p.go | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/server/p2p/p2p.go b/server/p2p/p2p.go
index 5db4fc1bf..e74544749 100644
--- a/server/p2p/p2p.go
+++ b/server/p2p/p2p.go
@@ -277,12 +277,17 @@ type Server struct {
 	relayInv              chan relayMsg
 	broadcast             chan broadcastMsg
 	wg                    sync.WaitGroup
-	quit                  chan struct{}
 	nat                   serverutils.NAT
 	db                    database.DB
 	TimeSource            blockdag.MedianTimeSource
 	services              wire.ServiceFlag
 
+	// We add to quitWaitGroup before every instance in which we wait for
+	// the quit channel so that all those instances finish before we shut
+	// down the managers (connManager, addrManager, etc),
+	quitWaitGroup sync.WaitGroup
+	quit          chan struct{}
+
 	// The following fields are used for optional indexes.  They will be nil
 	// if the associated index is not enabled.  These fields are set during
 	// initial creation of the server and never changed afterwards, so they
@@ -1167,6 +1172,8 @@ func (s *Server) peerHandler() {
 	s.addrManager.Start()
 	s.SyncManager.Start()
 
+	s.quitWaitGroup.Add(1)
+
 	srvrLog.Tracef("Starting peer handler")
 
 	state := &peerState{
@@ -1232,6 +1239,7 @@ out:
 				sp.Disconnect()
 				return true
 			})
+			s.quitWaitGroup.Done()
 			break out
 
 		case opcMsg := <-s.newOutboundConnection:
@@ -1239,6 +1247,10 @@ out:
 		}
 	}
 
+	// Wait for all p2p server quit jobs to finish before stopping the
+	// various managers
+	s.quitWaitGroup.Wait()
+
 	s.connManager.Stop()
 	s.SyncManager.Stop()
 	s.addrManager.Stop()
@@ -1341,6 +1353,8 @@ func (s *Server) rebroadcastHandler() {
 	timer := time.NewTimer(5 * time.Minute)
 	pendingInvs := make(map[wire.InvVect]interface{})
 
+	s.quitWaitGroup.Add(1)
+
 out:
 	for {
 		select {
@@ -1388,6 +1402,7 @@ cleanup:
 			break cleanup
 		}
 	}
+	s.quitWaitGroup.Done()
 	s.wg.Done()
 }
 
@@ -1525,6 +1540,9 @@ func (s *Server) upnpUpdateThread() {
 	timer := time.NewTimer(0 * time.Second)
 	lport, _ := strconv.ParseInt(config.ActiveConfig().NetParams().DefaultPort, 10, 16)
 	first := true
+
+	s.quitWaitGroup.Add(1)
+
 out:
 	for {
 		select {
@@ -1570,6 +1588,7 @@ out:
 		srvrLog.Debugf("successfully disestablished UPnP port mapping")
 	}
 
+	s.quitWaitGroup.Done()
 	s.wg.Done()
 }