From 61191a576dfff5505e4c0ea1527acd4ca26a2791 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Wed, 15 May 2024 14:27:45 -0700 Subject: [PATCH] peer: make PingManager.Stop infallible --- peer/brontide.go | 8 +++----- peer/ping_manager.go | 10 +++------- peer/ping_manager_test.go | 2 +- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/peer/brontide.go b/peer/brontide.go index 3cb682235..23e9dd834 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -1257,15 +1257,13 @@ func (p *Brontide) Disconnect(reason error) { p.log.Infof(err.Error()) + // Stop PingManager before closing TCP connection. + p.pingManager.Stop() + // Ensure that the TCP connection is properly closed before continuing. p.cfg.Conn.Close() close(p.quit) - - if err := p.pingManager.Stop(); err != nil { - p.log.Errorf("couldn't stop pingManager during disconnect: %v", - err) - } } // String returns the string representation of this peer. diff --git a/peer/ping_manager.go b/peer/ping_manager.go index c456a0581..f5c6180be 100644 --- a/peer/ping_manager.go +++ b/peer/ping_manager.go @@ -196,12 +196,10 @@ func (m *PingManager) pingHandler() { } } -// Stop interrupts the goroutines that the PingManager owns. Can only be called -// when the PingManager is running. -func (m *PingManager) Stop() error { +// Stop interrupts the goroutines that the PingManager owns. +func (m *PingManager) Stop() { if m.pingTicker == nil { - return errors.New("PingManager cannot be stopped because it " + - "isn't running") + return } m.stopped.Do(func() { @@ -211,8 +209,6 @@ func (m *PingManager) Stop() error { m.pingTicker.Stop() m.pingTimeout.Stop() }) - - return nil } // setPingState is a private method to keep track of all of the fields we need diff --git a/peer/ping_manager_test.go b/peer/ping_manager_test.go index bdfeeb6af..0f4c3be49 100644 --- a/peer/ping_manager_test.go +++ b/peer/ping_manager_test.go @@ -83,6 +83,6 @@ func TestPingManager(t *testing.T) { require.False(t, test.result) } - require.NoError(t, mgr.Stop(), "Could not stop pingManager") + mgr.Stop() } }