p2p/discover: don't attempt to replace nodes that are being replaced

PR #1621 changed Table locking so the mutex is not held while a
contested node is being pinged. If multiple nodes ping the local node
during this time window, multiple ping packets will be sent to the
contested node. The changes in this commit prevent multiple packets by
tracking whether the node is being replaced.

1 files changed, 11 insertions, 4 deletions

diff --git a/p2p/discover/table.go b/p2p/discover/table.go
index b077f010c..972bc1077 100644
--- a/p2p/discover/table.go
+++ b/p2p/discover/table.go
@@ -455,24 +455,31 @@ func (tab *Table) ping(id NodeID, addr *net.UDPAddr) error {
 func (tab *Table) add(new *Node) {
 	b := tab.buckets[logdist(tab.self.sha, new.sha)]
 	tab.mutex.Lock()
+	defer tab.mutex.Unlock()
 	if b.bump(new) {
-		tab.mutex.Unlock()
 		return
 	}
 	var oldest *Node
 	if len(b.entries) == bucketSize {
 		oldest = b.entries[bucketSize-1]
+		if oldest.contested {
+			// The node is already being replaced, don't attempt
+			// to replace it.
+			return
+		}
+		oldest.contested = true
 		// Let go of the mutex so other goroutines can access
 		// the table while we ping the least recently active node.
 		tab.mutex.Unlock()
-		if err := tab.ping(oldest.ID, oldest.addr()); err == nil {
+		err := tab.ping(oldest.ID, oldest.addr())
+		tab.mutex.Lock()
+		oldest.contested = false
+		if err == nil {
 			// The node responded, don't replace it.
 			return
 		}
-		tab.mutex.Lock()
 	}
 	added := b.replace(new, oldest)
-	tab.mutex.Unlock()
 	if added && tab.nodeAddedHook != nil {
 		tab.nodeAddedHook(new)
 	}