p2p: accept should not abort on first error (backport #8759) (#8760)

This commit is contained in:
mergify[bot]
2022-06-15 07:56:15 -04:00
committed by GitHub
parent d02f58e191
commit ce8284c027
2 changed files with 38 additions and 68 deletions

View File

@@ -576,14 +576,14 @@ func (r *Router) acceptPeers(transport Transport) {
ctx := r.stopCtx()
for {
conn, err := transport.Accept()
switch err {
case nil:
case io.EOF:
r.logger.Debug("stopping accept routine", "transport", transport)
switch {
case errors.Is(err, io.EOF):
r.logger.Debug("stopping accept routine", "transport", transport, "err", "EOF")
return
default:
case err != nil:
// in this case we got an error from the net.Listener.
r.logger.Error("failed to accept connection", "transport", transport, "err", err)
return
continue
}
incomingIP := conn.RemoteEndpoint().IP
@@ -595,7 +595,7 @@ func (r *Router) acceptPeers(transport Transport) {
"close_err", closeErr,
)
return
continue
}
// Spawn a goroutine for the handshake, to avoid head-of-line blocking.

View File

@@ -413,72 +413,42 @@ func TestRouter_AcceptPeers(t *testing.T) {
}
}
func TestRouter_AcceptPeers_Error(t *testing.T) {
t.Cleanup(leaktest.Check(t))
func TestRouter_AcceptPeers_Errors(t *testing.T) {
for _, err := range []error{io.EOF} {
t.Run(err.Error(), func(t *testing.T) {
t.Cleanup(leaktest.Check(t))
// Set up a mock transport that returns an error, which should prevent
// the router from calling Accept again.
mockTransport := &mocks.Transport{}
mockTransport.On("String").Maybe().Return("mock")
mockTransport.On("Protocols").Return([]p2p.Protocol{"mock"})
mockTransport.On("Accept").Once().Return(nil, errors.New("boom"))
mockTransport.On("Close").Return(nil)
// Set up a mock transport that returns io.EOF once, which should prevent
// the router from calling Accept again.
mockTransport := &mocks.Transport{}
mockTransport.On("String").Maybe().Return("mock")
mockTransport.On("Accept", mock.Anything).Once().Return(nil, err)
mockTransport.On("Listen", mock.Anything).Return(nil).Maybe()
mockTransport.On("Close").Return(nil)
mockTransport.On("Protocols").Return([]p2p.Protocol{"mock"})
// Set up and start the router.
peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{})
require.NoError(t, err)
// Set up and start the router.
peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{})
require.NoError(t, err)
defer peerManager.Close()
router, err := p2p.NewRouter(
log.TestingLogger(),
p2p.NopMetrics(),
selfInfo,
selfKey,
peerManager,
[]p2p.Transport{mockTransport},
p2p.RouterOptions{},
)
require.NoError(t, err)
router, err := p2p.NewRouter(
log.TestingLogger(),
p2p.NopMetrics(),
selfInfo,
selfKey,
peerManager,
[]p2p.Transport{mockTransport},
p2p.RouterOptions{},
)
require.NoError(t, err)
require.NoError(t, router.Start())
time.Sleep(time.Second)
require.NoError(t, router.Stop())
require.NoError(t, router.Start())
time.Sleep(time.Second)
require.NoError(t, router.Stop())
mockTransport.AssertExpectations(t)
mockTransport.AssertExpectations(t)
}
func TestRouter_AcceptPeers_ErrorEOF(t *testing.T) {
t.Cleanup(leaktest.Check(t))
// Set up a mock transport that returns io.EOF once, which should prevent
// the router from calling Accept again.
mockTransport := &mocks.Transport{}
mockTransport.On("String").Maybe().Return("mock")
mockTransport.On("Protocols").Return([]p2p.Protocol{"mock"})
mockTransport.On("Accept").Once().Return(nil, io.EOF)
mockTransport.On("Close").Return(nil)
// Set up and start the router.
peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{})
require.NoError(t, err)
defer peerManager.Close()
router, err := p2p.NewRouter(
log.TestingLogger(),
p2p.NopMetrics(),
selfInfo,
selfKey,
peerManager,
[]p2p.Transport{mockTransport},
p2p.RouterOptions{},
)
require.NoError(t, err)
require.NoError(t, router.Start())
time.Sleep(time.Second)
require.NoError(t, router.Stop())
mockTransport.AssertExpectations(t)
})
}
}
func TestRouter_AcceptPeers_HeadOfLineBlocking(t *testing.T) {