From b5cbeb4ad788f606df1aa2e61853383e99267c2f Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 15 Jan 2024 13:25:27 -0800 Subject: [PATCH] peer: make PingManager disconnect call async In this commit, we make all calls to disconnect after a ping/pong violation is detected in the `PingManager` async. We do this to avoid circular waiting that may occur if the disconnect call back ends up waiting on the peer goroutine to be torn down. If this happens, then the peer goroutine will be blocked on the ping manager fully tearing down, which is blocked on the peer disconnect succeeding. This is a similar class of issue we've delt with recently as pertains to the peer and the server: sync all back execution must not lead to a circular waiting loop. Fixes #8379 --- peer/brontide.go | 2 +- peer/ping_manager.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/peer/brontide.go b/peer/brontide.go index 1611ff633..a97c1f438 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -577,7 +577,7 @@ func NewBrontide(cfg Config) *Brontide { eStr := "pong response failure for %s: %v " + "-- disconnecting" p.log.Warnf(eStr, p, err) - p.Disconnect(fmt.Errorf(eStr, p, err)) + go p.Disconnect(fmt.Errorf(eStr, p, err)) }, }) diff --git a/peer/ping_manager.go b/peer/ping_manager.go index 3e62cb097..b840d47c4 100644 --- a/peer/ping_manager.go +++ b/peer/ping_manager.go @@ -13,7 +13,6 @@ import ( // PingManagerConfig is a structure containing various parameters that govern // how the PingManager behaves. type PingManagerConfig struct { - // NewPingPayload is a closure that returns the payload to be packaged // in the Ping message. NewPingPayload func() []byte @@ -144,6 +143,7 @@ func (m *PingManager) start() error { // Set up our bookkeeping for the new Ping. if err := m.setPingState(pongSize); err != nil { + m.cfg.OnPongFailure(err) return @@ -157,6 +157,7 @@ func (m *PingManager) start() error { e := errors.New("timeout while waiting for " + "pong response", ) + m.cfg.OnPongFailure(e) return @@ -177,6 +178,7 @@ func (m *PingManager) start() error { e := errors.New("pong response does " + "not match expected size", ) + m.cfg.OnPongFailure(e) return @@ -188,6 +190,7 @@ func (m *PingManager) start() error { rtt := time.Since(*lastPing) m.pingTime.Store(&rtt) } + case <-m.quit: return }