Prevent potential call on discv5 which became nil

It could happen when we found multiple relevant peers and they were
processed before discv5 closed. For example we have max limit of 1:
- found peer A and B in the same kademlia cycle
- process peer A
- max limit is reached -> closed discv5 and set it to nil
- process peer B
- panic!!!
This commit is contained in:
Dmitry Shulyak 2018-04-16 21:15:03 +03:00 committed by Dmitry Shulyak
parent 6c9ec5b337
commit fa526e7444
2 changed files with 42 additions and 7 deletions

View File

@ -152,18 +152,27 @@ func (p *PeerPool) handleServerPeers(server *p2p.Server, events <-chan *p2p.Peer
retryDiscv5 = time.After(0)
}
case p2p.PeerEventTypeAdd:
log.Debug("confirm peer added", "ID", event.Peer)
if p.stopOnMax && p.handleAddedPeer(server, event.Peer) {
log.Debug("closing discv5 connection", "server", server.Self())
server.DiscV5.Close()
server.DiscV5 = nil
p.feed.Send(Discv5Closed)
}
p.handleAddedEvent(server, event)
}
}
}
}
// handledAddedEvent maintains logic of stopping discovery server and added mainly
// for easier testing.
func (p *PeerPool) handleAddedEvent(server *p2p.Server, event *p2p.PeerEvent) {
log.Debug("confirm peer added", "ID", event.Peer)
if p.stopOnMax && p.handleAddedPeer(server, event.Peer) {
if server.DiscV5 == nil {
return
}
log.Debug("closing discv5 connection", "server", server.Self())
server.DiscV5.Close()
server.DiscV5 = nil
p.feed.Send(Discv5Closed)
}
}
// handleAddedPeer notifies all topics about added peer and return true if all topics has max limit of connections
func (p *PeerPool) handleAddedPeer(server *p2p.Server, nodeID discover.NodeID) (all bool) {
p.mu.Lock()

View File

@ -12,6 +12,7 @@ import (
"github.com/ethereum/go-ethereum/p2p"
"github.com/ethereum/go-ethereum/p2p/discover"
"github.com/ethereum/go-ethereum/p2p/discv5"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/status-im/status-go/geth/params"
@ -133,3 +134,28 @@ func (s *PeerPoolSimulationSuite) TearDown() {
p.Stop()
}
}
// TestMaxPeersOverflow verifies that following scenario will not occur:
// - found peer A and B in the same kademlia cycle
// - process peer A
// - max limit is reached -> closed discv5 and set it to nil
// - process peer B
// - panic because discv5 is nil!!!
func TestMaxPeersOverflow(t *testing.T) {
key, err := crypto.GenerateKey()
require.NoError(t, err)
peer := &p2p.Server{
Config: p2p.Config{
PrivateKey: key,
DiscoveryV5: true,
NoDiscovery: true,
},
}
require.NoError(t, peer.Start())
defer peer.Stop()
require.NotNil(t, peer.DiscV5)
pool := NewPeerPool(nil, DefaultFastSync, DefaultSlowSync, nil, true)
pool.handleAddedEvent(peer, &p2p.PeerEvent{})
require.Nil(t, peer.DiscV5)
pool.handleAddedEvent(peer, &p2p.PeerEvent{})
}