From a04615cbabcf314ba5f0581fa8d1e0d02d39fd60 Mon Sep 17 00:00:00 2001 From: Rafael Sampaio <5679073+r4f4ss@users.noreply.github.com> Date: Sat, 5 Apr 2025 22:54:27 -0300 Subject: [PATCH 1/2] discover:add mutex and lock/unlock logics for revalidationList.nextTime --- p2p/discover/table_reval.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/p2p/discover/table_reval.go b/p2p/discover/table_reval.go index 1519313d19..932ab1fd38 100644 --- a/p2p/discover/table_reval.go +++ b/p2p/discover/table_reval.go @@ -20,6 +20,7 @@ import ( "fmt" "math" "slices" + "sync" "time" "github.com/ethereum/go-ethereum/common/mclock" @@ -78,18 +79,29 @@ func (tr *tableRevalidation) nodeEndpointChanged(tab *Table, n *tableNode) { // to schedule a timer. However, run can be called at any time. func (tr *tableRevalidation) run(tab *Table, now mclock.AbsTime) (nextTime mclock.AbsTime) { reval := func(list *revalidationList) { + // nextTime locked for reading, unlock in the end of function or before schedule to prevent deadlock + list.ntLock.RLock() if list.nextTime <= now { if n := list.get(&tab.rand, tr.activeReq); n != nil { tr.startRequest(tab, n) } // Update nextTime regardless if any requests were started because // current value has passed. + // unlock before schedule to prevent deadlock with write lock, return to prevent double unlock + list.ntLock.RUnlock() list.schedule(now, &tab.rand) + return } + // in case of false if verification, unlock nextTime + list.ntLock.RUnlock() } reval(&tr.fast) reval(&tr.slow) + tr.fast.ntLock.RLock() + tr.slow.ntLock.RLock() + defer tr.fast.ntLock.RUnlock() + defer tr.slow.ntLock.RUnlock() return min(tr.fast.nextTime, tr.slow.nextTime) } @@ -198,6 +210,7 @@ func (tr *tableRevalidation) moveToList(dest *revalidationList, n *tableNode, no type revalidationList struct { nodes []*tableNode nextTime mclock.AbsTime + ntLock sync.RWMutex interval time.Duration name string } @@ -218,6 +231,8 @@ func (list *revalidationList) get(rand randomSource, exclude map[enode.ID]struct } func (list *revalidationList) schedule(now mclock.AbsTime, rand randomSource) { + list.ntLock.Lock() + defer list.ntLock.Unlock() list.nextTime = now.Add(time.Duration(rand.Int63n(int64(list.interval)))) } @@ -236,6 +251,8 @@ func (list *revalidationList) remove(n *tableNode) { } list.nodes = slices.Delete(list.nodes, i, i+1) if len(list.nodes) == 0 { + list.ntLock.Lock() + defer list.ntLock.Unlock() list.nextTime = never } n.revalList = nil From 862808047542a522b8be224b0a5e8fa1b122b868 Mon Sep 17 00:00:00 2001 From: Rafael Sampaio <5679073+r4f4ss@users.noreply.github.com> Date: Mon, 7 Apr 2025 20:01:34 -0300 Subject: [PATCH 2/2] discover: replace created locks by table lock on run function --- p2p/discover/table_reval.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/p2p/discover/table_reval.go b/p2p/discover/table_reval.go index 932ab1fd38..d5135bd9eb 100644 --- a/p2p/discover/table_reval.go +++ b/p2p/discover/table_reval.go @@ -20,7 +20,6 @@ import ( "fmt" "math" "slices" - "sync" "time" "github.com/ethereum/go-ethereum/common/mclock" @@ -79,29 +78,20 @@ func (tr *tableRevalidation) nodeEndpointChanged(tab *Table, n *tableNode) { // to schedule a timer. However, run can be called at any time. func (tr *tableRevalidation) run(tab *Table, now mclock.AbsTime) (nextTime mclock.AbsTime) { reval := func(list *revalidationList) { - // nextTime locked for reading, unlock in the end of function or before schedule to prevent deadlock - list.ntLock.RLock() if list.nextTime <= now { if n := list.get(&tab.rand, tr.activeReq); n != nil { tr.startRequest(tab, n) } // Update nextTime regardless if any requests were started because // current value has passed. - // unlock before schedule to prevent deadlock with write lock, return to prevent double unlock - list.ntLock.RUnlock() + tab.mutex.Lock() list.schedule(now, &tab.rand) - return + tab.mutex.Unlock() } - // in case of false if verification, unlock nextTime - list.ntLock.RUnlock() } reval(&tr.fast) reval(&tr.slow) - tr.fast.ntLock.RLock() - tr.slow.ntLock.RLock() - defer tr.fast.ntLock.RUnlock() - defer tr.slow.ntLock.RUnlock() return min(tr.fast.nextTime, tr.slow.nextTime) } @@ -210,7 +200,6 @@ func (tr *tableRevalidation) moveToList(dest *revalidationList, n *tableNode, no type revalidationList struct { nodes []*tableNode nextTime mclock.AbsTime - ntLock sync.RWMutex interval time.Duration name string } @@ -231,8 +220,6 @@ func (list *revalidationList) get(rand randomSource, exclude map[enode.ID]struct } func (list *revalidationList) schedule(now mclock.AbsTime, rand randomSource) { - list.ntLock.Lock() - defer list.ntLock.Unlock() list.nextTime = now.Add(time.Duration(rand.Int63n(int64(list.interval)))) } @@ -251,8 +238,6 @@ func (list *revalidationList) remove(n *tableNode) { } list.nodes = slices.Delete(list.nodes, i, i+1) if len(list.nodes) == 0 { - list.ntLock.Lock() - defer list.ntLock.Unlock() list.nextTime = never } n.revalList = nil