mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-08-20 10:52:25 +00:00
p2p/simulations: fix deadlock during network shutdown
`TestDiscoveryPersistenceSimulationSimAdapter` often got into deadlock. The execution was stuck on two locks, i.e, `Kademlia.lock` and `p2p/simulations.Network.lock`. Usually the test got stuck once in each 20 executions with high confidence. `Kademlia` was stuck in `Kademlia.EachAddr()` and `Network` in `Network.Stop()`. Solution: in `Network.Stop()` `net.lock` must be released before calling `node.Stop()` as stopping a node (somehow - I did not find the exact code path) causes `Network.InitConn()` to be called from `Kademlia.SuggestPeer()` and that blocks on `net.lock`. Related ticket: https://github.com/ethersphere/go-ethereum/issues/1223
This commit is contained in:
parent
ddfe26cbfa
commit
c35c7ed718
1 changed files with 27 additions and 9 deletions
|
|
@ -253,18 +253,36 @@ func (net *Network) watchPeerEvents(id enode.ID, events chan *p2p.PeerEvent, sub
|
||||||
|
|
||||||
// Stop stops the node with the given ID
|
// Stop stops the node with the given ID
|
||||||
func (net *Network) Stop(id enode.ID) error {
|
func (net *Network) Stop(id enode.ID) error {
|
||||||
|
// IMPORTANT: node.Stop() must NOT be called under net.lock as
|
||||||
|
// node.Reachable() closure has a reference to the network and
|
||||||
|
// calls net.InitConn() what also locks the network. => DEADLOCK
|
||||||
|
// That holds until the following ticket is not resolved:
|
||||||
|
|
||||||
|
var err error
|
||||||
|
|
||||||
|
node, err := func() (*Node, error) {
|
||||||
|
net.lock.Lock()
|
||||||
|
defer net.lock.Unlock()
|
||||||
|
|
||||||
|
node := net.getNode(id)
|
||||||
|
if node == nil {
|
||||||
|
return nil, fmt.Errorf("node %v does not exist", id)
|
||||||
|
}
|
||||||
|
if !node.Up() {
|
||||||
|
return nil, fmt.Errorf("node %v already down", id)
|
||||||
|
}
|
||||||
|
node.SetUp(false)
|
||||||
|
return node, nil
|
||||||
|
}()
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
err = node.Stop() // must be called without net.lock
|
||||||
|
|
||||||
net.lock.Lock()
|
net.lock.Lock()
|
||||||
defer net.lock.Unlock()
|
defer net.lock.Unlock()
|
||||||
node := net.getNode(id)
|
|
||||||
if node == nil {
|
|
||||||
return fmt.Errorf("node %v does not exist", id)
|
|
||||||
}
|
|
||||||
if !node.Up() {
|
|
||||||
return fmt.Errorf("node %v already down", id)
|
|
||||||
}
|
|
||||||
node.SetUp(false)
|
|
||||||
|
|
||||||
err := node.Stop()
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
node.SetUp(true)
|
node.SetUp(true)
|
||||||
return err
|
return err
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue