From a148fa797a732aefd189514b666cab11bab914c0 Mon Sep 17 00:00:00 2001
From: Jonathan Chappelow <chappjc@protonmail.com>
Date: Wed, 20 Oct 2021 11:59:18 -0500
Subject: [PATCH] addrmgr: make KnownAddress methods thread-safe

This gives KnownAddress a sync.RWMutex so the exported methods may
safely access the na (*wire.NetAddress) and lastattempt fields.
The AddrManager is updated to lock the new KnownAddress mutex before
assigning to na or lastattempt.
The other KnownAddress fields are only accessed by AddrManager, using
its own Mutex for synchronization.
---
 addrmgr/addrmanager.go  | 17 ++++++++++++++---
 addrmgr/knownaddress.go | 11 +++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/addrmgr/addrmanager.go b/addrmgr/addrmanager.go
index fa8f27bc..4c737b93 100644
--- a/addrmgr/addrmanager.go
+++ b/addrmgr/addrmanager.go
@@ -176,7 +176,7 @@ func (a *AddrManager) updateAddress(netAddr, srcAddr *wire.NetAddress) {
 		// TODO: only update addresses periodically.
 		// Update the last seen time and services.
 		// note that to prevent causing excess garbage on getaddr
-		// messages the netaddresses in addrmaanger are *immutable*,
+		// messages the netaddresses in addrmanager are *immutable*,
 		// if we need to change them then we replace the pointer with a
 		// new copy so that we don't have to copy every na for getaddr.
 		if netAddr.Timestamp.After(ka.na.Timestamp) ||
@@ -186,7 +186,9 @@ func (a *AddrManager) updateAddress(netAddr, srcAddr *wire.NetAddress) {
 			naCopy := *ka.na
 			naCopy.Timestamp = netAddr.Timestamp
 			naCopy.AddService(netAddr.Services)
+			ka.mtx.Lock()
 			ka.na = &naCopy
+			ka.mtx.Unlock()
 		}
 
 		// If already in tried, we have nothing to do here.
@@ -857,8 +859,11 @@ func (a *AddrManager) Attempt(addr *wire.NetAddress) {
 		return
 	}
 	// set last tried time to now
+	now := time.Now()
+	ka.mtx.Lock()
 	ka.attempts++
-	ka.lastattempt = time.Now()
+	ka.lastattempt = now
+	ka.mtx.Unlock()
 }
 
 // Connected Marks the given address as currently connected and working at the
@@ -880,7 +885,9 @@ func (a *AddrManager) Connected(addr *wire.NetAddress) {
 		// ka.na is immutable, so replace it.
 		naCopy := *ka.na
 		naCopy.Timestamp = time.Now()
+		ka.mtx.Lock()
 		ka.na = &naCopy
+		ka.mtx.Unlock()
 	}
 }
 
@@ -899,11 +906,13 @@ func (a *AddrManager) Good(addr *wire.NetAddress) {
 	// ka.Timestamp is not updated here to avoid leaking information
 	// about currently connected peers.
 	now := time.Now()
+	ka.mtx.Lock()
 	ka.lastsuccess = now
 	ka.lastattempt = now
 	ka.attempts = 0
+	ka.mtx.Unlock() // tried and refs synchronized via a.mtx
 
-	// move to tried set, optionally evicting other addresses if neeed.
+	// move to tried set, optionally evicting other addresses if need.
 	if ka.tried {
 		return
 	}
@@ -988,7 +997,9 @@ func (a *AddrManager) SetServices(addr *wire.NetAddress, services wire.ServiceFl
 		// ka.na is immutable, so replace it.
 		naCopy := *ka.na
 		naCopy.Services = services
+		ka.mtx.Lock()
 		ka.na = &naCopy
+		ka.mtx.Unlock()
 	}
 }
 
diff --git a/addrmgr/knownaddress.go b/addrmgr/knownaddress.go
index 15469f37..5a7674f4 100644
--- a/addrmgr/knownaddress.go
+++ b/addrmgr/knownaddress.go
@@ -5,6 +5,7 @@
 package addrmgr
 
 import (
+	"sync"
 	"time"
 
 	"github.com/btcsuite/btcd/wire"
@@ -13,6 +14,7 @@ import (
 // KnownAddress tracks information about a known network address that is used
 // to determine how viable an address is.
 type KnownAddress struct {
+	mtx         sync.RWMutex // na and lastattempt
 	na          *wire.NetAddress
 	srcAddr     *wire.NetAddress
 	attempts    int
@@ -25,19 +27,28 @@ type KnownAddress struct {
 // NetAddress returns the underlying wire.NetAddress associated with the
 // known address.
 func (ka *KnownAddress) NetAddress() *wire.NetAddress {
+	ka.mtx.RLock()
+	defer ka.mtx.RUnlock()
 	return ka.na
 }
 
 // LastAttempt returns the last time the known address was attempted.
 func (ka *KnownAddress) LastAttempt() time.Time {
+	ka.mtx.RLock()
+	defer ka.mtx.RUnlock()
 	return ka.lastattempt
 }
 
 // Services returns the services supported by the peer with the known address.
 func (ka *KnownAddress) Services() wire.ServiceFlag {
+	ka.mtx.RLock()
+	defer ka.mtx.RUnlock()
 	return ka.na.Services
 }
 
+// The unexported methods, chance and isBad, are used from within AddrManager
+// where KnownAddress field access is synchronized via it's own Mutex.
+
 // chance returns the selection probability for a known address.  The priority
 // depends upon how recently the address has been seen, how recently it was last
 // attempted and how often attempts to connect to it have failed.