From 100ff08de93ff1907bf810f584ec5bdc7a2a5260 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 11 Mar 2019 15:31:53 +0400 Subject: [PATCH] p2p: do not panic when filter times out (#3384) Fixes #3369 --- CHANGELOG_PENDING.md | 1 + p2p/switch.go | 9 ++++++++- p2p/switch_test.go | 39 +++++++++++++++++++++++++++++++++++++++ p2p/transport.go | 2 +- 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 9ca5ab649..5120e2632 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -46,3 +46,4 @@ CI/CD: * [\#3396](https://github.com/tendermint/tendermint/pull/3396) - [p2p/conn] \#3347 Reject all-zero shared secrets in the Diffie-Hellman step of secret-connection - [libs/pubsub] \#951, \#1880 use non-blocking send when dispatching messages [ADR-33](https://github.com/tendermint/tendermint/blob/develop/docs/architecture/adr-033-pubsub.md) +- [p2p] \#3369 do not panic when filter times out diff --git a/p2p/switch.go b/p2p/switch.go index ccd6d40f2..a07f70ce9 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -497,7 +497,14 @@ func (sw *Switch) acceptRoutine() { ) continue - case *ErrTransportClosed: + case ErrFilterTimeout: + sw.Logger.Error( + "Peer filter timed out", + "err", err, + ) + + continue + case ErrTransportClosed: sw.Logger.Error( "Stopped accept routine, as transport is closed", "numPeers", sw.peers.Size(), diff --git a/p2p/switch_test.go b/p2p/switch_test.go index 47cfed55f..d5dd178b6 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -2,6 +2,7 @@ package p2p import ( "bytes" + "errors" "fmt" "io" "io/ioutil" @@ -532,6 +533,44 @@ func TestSwitchAcceptRoutine(t *testing.T) { } } +type errorTransport struct { + acceptErr error +} + +func (et errorTransport) Accept(c peerConfig) (Peer, error) { + return nil, et.acceptErr +} +func (errorTransport) Dial(NetAddress, peerConfig) (Peer, error) { + panic("not implemented") +} +func (errorTransport) Cleanup(Peer) { + panic("not implemented") +} + +func TestSwitchAcceptRoutineErrorCases(t *testing.T) { + sw := NewSwitch(cfg, errorTransport{ErrFilterTimeout{}}) + assert.NotPanics(t, func() { + err := sw.Start() + assert.NoError(t, err) + sw.Stop() + }) + + sw = NewSwitch(cfg, errorTransport{ErrRejected{conn: nil, err: errors.New("filtered"), isFiltered: true}}) + assert.NotPanics(t, func() { + err := sw.Start() + assert.NoError(t, err) + sw.Stop() + }) + // TODO(melekes) check we remove our address from addrBook + + sw = NewSwitch(cfg, errorTransport{ErrTransportClosed{}}) + assert.NotPanics(t, func() { + err := sw.Start() + assert.NoError(t, err) + sw.Stop() + }) +} + func BenchmarkSwitchBroadcast(b *testing.B) { s1, s2 := MakeSwitchPair(b, func(i int, sw *Switch) *Switch { // Make bar reactors of bar channels each diff --git a/p2p/transport.go b/p2p/transport.go index d1bccf9b8..d36065ab1 100644 --- a/p2p/transport.go +++ b/p2p/transport.go @@ -175,7 +175,7 @@ func (mt *MultiplexTransport) Accept(cfg peerConfig) (Peer, error) { return mt.wrapPeer(a.conn, a.nodeInfo, cfg, nil), nil case <-mt.closec: - return nil, &ErrTransportClosed{} + return nil, ErrTransportClosed{} } }