addrmgr: make KnownAddress methods thread-safe

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()
 	}
 }
 

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.