From 0dbda6d97f1e7115edff9c6a599d25c616ca03fe Mon Sep 17 00:00:00 2001 From: Jorropo Date: Thu, 15 Jun 2023 02:38:55 +0200 Subject: [PATCH 1/3] fix: fix abba bug in UsefullNewPeer This fixes a deadlock the issue is that this pattern could happen because .Find inside UsefullNewPeer would aquire the same RLock: - UsefullNewPeer takes tablock.RLock - Something else tries to take tablock.Lock (waits) - UsefullNewPeer calls Find calls NearestPeer call NearestPeers which trie to take tablock.RLock again, here RLock waits because the rwmutex is starving, it will wait for the Something else to aqcuire the Lock first else a constant flow of reader would block the RWMutex forever. --- table.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/table.go b/table.go index 667e30c..2791a1d 100644 --- a/table.go +++ b/table.go @@ -120,7 +120,7 @@ func (rt *RoutingTable) UsefulNewPeer(p peer.ID) bool { rt.tabLock.RLock() defer rt.tabLock.RUnlock() - if rt.Find(p) != "" { + if rt.find(p) != "" { // peer already exists in the routing table, so it isn't useful return false } @@ -412,12 +412,21 @@ func (rt *RoutingTable) nextBucket() { } // Find a specific peer by ID or return nil -func (rt *RoutingTable) Find(id peer.ID) peer.ID { - srch := rt.NearestPeers(ConvertPeerID(id), 1) - if len(srch) == 0 || srch[0] != id { +func (rt *RoutingTable) Find(p peer.ID) peer.ID { + rt.tabLock.RLock() + defer rt.tabLock.RUnlock() + + return rt.find(p) +} + +// caller is responsible for calling +func (rt *RoutingTable) find(p peer.ID) peer.ID { + pi := rt.buckets[rt.bucketIdForPeer(p)].getPeer(p) + if pi == nil { return "" } - return srch[0] + + return p } // NearestPeer returns a single peer that is nearest to the given ID From e99cd472ed1ed9f24acd6142c88614a4c3ebbe70 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Thu, 15 Jun 2023 02:41:29 +0200 Subject: [PATCH 2/3] chore: release v0.6.3 --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index b069fc6..8a508ec 100644 --- a/version.json +++ b/version.json @@ -1,3 +1,3 @@ { - "version": "v0.6.2" + "version": "v0.6.3" } From d7ca005e8a3e25fcdafd40e30adc3ba856462fab Mon Sep 17 00:00:00 2001 From: guillaumemichel Date: Thu, 15 Jun 2023 07:46:11 +0200 Subject: [PATCH 3/3] reverted to old Find(), and updated find mechanism in UsefulNewPeer() --- table.go | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/table.go b/table.go index 2791a1d..4457cf9 100644 --- a/table.go +++ b/table.go @@ -120,15 +120,15 @@ func (rt *RoutingTable) UsefulNewPeer(p peer.ID) bool { rt.tabLock.RLock() defer rt.tabLock.RUnlock() - if rt.find(p) != "" { - // peer already exists in the routing table, so it isn't useful - return false - } - // bucket corresponding to p bucketID := rt.bucketIdForPeer(p) bucket := rt.buckets[bucketID] + if bucket.getPeer(p) != nil { + // peer already exists in the routing table, so it isn't useful + return false + } + // bucket isn't full if bucket.len() < rt.bucketsize { return true @@ -412,21 +412,12 @@ func (rt *RoutingTable) nextBucket() { } // Find a specific peer by ID or return nil -func (rt *RoutingTable) Find(p peer.ID) peer.ID { - rt.tabLock.RLock() - defer rt.tabLock.RUnlock() - - return rt.find(p) -} - -// caller is responsible for calling -func (rt *RoutingTable) find(p peer.ID) peer.ID { - pi := rt.buckets[rt.bucketIdForPeer(p)].getPeer(p) - if pi == nil { +func (rt *RoutingTable) Find(id peer.ID) peer.ID { + srch := rt.NearestPeers(ConvertPeerID(id), 1) + if len(srch) == 0 || srch[0] != id { return "" } - - return p + return srch[0] } // NearestPeer returns a single peer that is nearest to the given ID