From adedf76f3e6c1eb17fefcd856c2d10e46ac2d98e Mon Sep 17 00:00:00 2001 From: niuxiaojie81 <85773309@qq.com> Date: Fri, 29 Dec 2023 10:41:56 +0800 Subject: [PATCH 1/3] eth: avoid blocking when geth is closed, fix hang in waitSnapExtension --- eth/peerset.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/eth/peerset.go b/eth/peerset.go index b27d3964a119..cc4440489873 100644 --- a/eth/peerset.go +++ b/eth/peerset.go @@ -21,6 +21,7 @@ import ( "fmt" "math/big" "sync" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/eth/protocols/eth" @@ -28,6 +29,11 @@ import ( "github.com/ethereum/go-ethereum/p2p" ) +const ( + // snapWaitTimeout is the amount of time to wait for the snap protocol to be started. + snapWaitTimeout = 5 * time.Second +) + var ( // errPeerSetClosed is returned if a peer is attempted to be added or removed // from the peer set after it has been terminated. @@ -44,6 +50,9 @@ var ( // errSnapWithoutEth is returned if a peer attempts to connect only on the // snap protocol without advertising the eth main protocol. errSnapWithoutEth = errors.New("peer connected on snap without compatible eth support") + + // errSnapTimeout is returned if the peer takes too long to start the snap protocol. + errSnapTimeout = errors.New("peer timeout starting snap protocol") ) // peerSet represents the collection of active peers currently participating in @@ -129,7 +138,21 @@ func (ps *peerSet) waitSnapExtension(peer *eth.Peer) (*snap.Peer, error) { ps.snapWait[id] = wait ps.lock.Unlock() - return <-wait, nil + t := time.NewTicker(snapWaitTimeout) + defer t.Stop() + for { + select { + case p := <-wait: + return p, nil + case <-t.C: + if ps.closed { + ps.lock.Lock() + delete(ps.snapWait, id) + ps.lock.Unlock() + return nil, errSnapTimeout + } + } + } } // registerPeer injects a new `eth` peer into the working set, or returns an error From 14a2915930a5dbbbfde95eea1469ba7114df700a Mon Sep 17 00:00:00 2001 From: niuxiaojie81 <85773309@qq.com> Date: Wed, 10 Jan 2024 11:46:23 +0800 Subject: [PATCH 2/3] eth: avoid blocking when geth is closed, fix hang in waitSnapExtension --- eth/peerset.go | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/eth/peerset.go b/eth/peerset.go index cc4440489873..9b631497fd55 100644 --- a/eth/peerset.go +++ b/eth/peerset.go @@ -21,7 +21,6 @@ import ( "fmt" "math/big" "sync" - "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/eth/protocols/eth" @@ -29,11 +28,6 @@ import ( "github.com/ethereum/go-ethereum/p2p" ) -const ( - // snapWaitTimeout is the amount of time to wait for the snap protocol to be started. - snapWaitTimeout = 5 * time.Second -) - var ( // errPeerSetClosed is returned if a peer is attempted to be added or removed // from the peer set after it has been terminated. @@ -50,9 +44,6 @@ var ( // errSnapWithoutEth is returned if a peer attempts to connect only on the // snap protocol without advertising the eth main protocol. errSnapWithoutEth = errors.New("peer connected on snap without compatible eth support") - - // errSnapTimeout is returned if the peer takes too long to start the snap protocol. - errSnapTimeout = errors.New("peer timeout starting snap protocol") ) // peerSet represents the collection of active peers currently participating in @@ -66,6 +57,7 @@ type peerSet struct { lock sync.RWMutex closed bool + quitCh chan struct{} // Quit channel to signal termination } // newPeerSet creates a new peer set to track the active participants. @@ -74,6 +66,7 @@ func newPeerSet() *peerSet { peers: make(map[string]*ethPeer), snapWait: make(map[string]chan *snap.Peer), snapPend: make(map[string]*snap.Peer), + quitCh: make(chan struct{}), } } @@ -138,19 +131,15 @@ func (ps *peerSet) waitSnapExtension(peer *eth.Peer) (*snap.Peer, error) { ps.snapWait[id] = wait ps.lock.Unlock() - t := time.NewTicker(snapWaitTimeout) - defer t.Stop() for { select { case p := <-wait: return p, nil - case <-t.C: - if ps.closed { - ps.lock.Lock() - delete(ps.snapWait, id) - ps.lock.Unlock() - return nil, errSnapTimeout - } + case <-ps.quitCh: + ps.lock.Lock() + delete(ps.snapWait, id) + ps.lock.Unlock() + return nil, errPeerSetClosed } } } @@ -279,5 +268,6 @@ func (ps *peerSet) close() { for _, p := range ps.peers { p.Disconnect(p2p.DiscQuitting) } + close(ps.quitCh) ps.closed = true } From f05fcaa4899d4c310511b5bd8371ab09d81318b2 Mon Sep 17 00:00:00 2001 From: niuxiaojie81 <85773309@qq.com> Date: Mon, 15 Jan 2024 10:31:41 +0800 Subject: [PATCH 3/3] eth: avoid blocking when geth is closed, fix hang in waitSnapExtension --- eth/peerset.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/eth/peerset.go b/eth/peerset.go index 9b631497fd55..c0c11e3e85ee 100644 --- a/eth/peerset.go +++ b/eth/peerset.go @@ -131,16 +131,14 @@ func (ps *peerSet) waitSnapExtension(peer *eth.Peer) (*snap.Peer, error) { ps.snapWait[id] = wait ps.lock.Unlock() - for { - select { - case p := <-wait: - return p, nil - case <-ps.quitCh: - ps.lock.Lock() - delete(ps.snapWait, id) - ps.lock.Unlock() - return nil, errPeerSetClosed - } + select { + case p := <-wait: + return p, nil + case <-ps.quitCh: + ps.lock.Lock() + delete(ps.snapWait, id) + ps.lock.Unlock() + return nil, errPeerSetClosed } } @@ -268,6 +266,8 @@ func (ps *peerSet) close() { for _, p := range ps.peers { p.Disconnect(p2p.DiscQuitting) } - close(ps.quitCh) + if !ps.closed { + close(ps.quitCh) + } ps.closed = true }