From 7761da5ae561a124d3de776b6fe4743edcbd0abf Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 12 Dec 2022 08:20:37 +0000 Subject: [PATCH] chore(all): remove unneeded timer channel drains --- dot/network/discovery.go | 4 +--- dot/network/notifications.go | 4 +--- dot/network/transaction.go | 4 +--- internal/metrics/metrics.go | 4 +--- lib/babe/babe.go | 22 ++++++---------------- lib/babe/epoch_handler.go | 12 +++--------- lib/babe/epoch_handler_test.go | 4 +--- lib/grandpa/finalisation.go | 15 +++------------ tests/rpc/system_integration_test.go | 4 +--- tests/stress/helpers.go | 4 +--- tests/stress/stress_test.go | 4 +--- 11 files changed, 20 insertions(+), 61 deletions(-) diff --git a/dot/network/discovery.go b/dot/network/discovery.go index fa6b0ab613..e8cb8212c1 100644 --- a/dot/network/discovery.go +++ b/dot/network/discovery.go @@ -155,9 +155,7 @@ func (d *discovery) advertise() { select { case <-d.ctx.Done(): - if !timer.Stop() { - <-timer.C - } + timer.Stop() return case <-timer.C: logger.Debug("advertising ourselves in the DHT...") diff --git a/dot/network/notifications.go b/dot/network/notifications.go index 5f59e6268d..1dcab79157 100644 --- a/dot/network/notifications.go +++ b/dot/network/notifications.go @@ -362,9 +362,7 @@ func (s *Service) sendHandshake(peer peer.ID, hs Handshake, info *notificationsP closeOutboundStream(info, peer, stream) return nil, errHandshakeTimeout case hsResponse := <-s.readHandshake(stream, info.handshakeDecoder, info.maxSize): - if !hsTimer.Stop() { - <-hsTimer.C - } + hsTimer.Stop() if hsResponse.err != nil { logger.Tracef("failed to read handshake from peer %s using protocol %s: %s", peer, info.protocolID, hsResponse.err) diff --git a/dot/network/transaction.go b/dot/network/transaction.go index 32c745bd55..e9b55ec41b 100644 --- a/dot/network/transaction.go +++ b/dot/network/transaction.go @@ -146,9 +146,7 @@ func (s *Service) createBatchMessageHandler(txnBatchCh chan *batchMessage) Notif select { case txnBatchCh <- data: - if !timer.Stop() { - <-timer.C - } + timer.Stop() case <-timer.C: logger.Debugf("transaction message %s for peer %s not included into batch", msg, peer) } diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index e8f3ef43d6..d67ed5ccdc 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -85,9 +85,7 @@ func (s *Server) Stop() (err error) { select { case err := <-s.done: close(s.done) - if !timeout.Stop() { - <-timeout.C - } + timeout.Stop() if err != nil { return err } diff --git a/lib/babe/babe.go b/lib/babe/babe.go index f6c4ae39a8..f3ec675a01 100644 --- a/lib/babe/babe.go +++ b/lib/babe/babe.go @@ -216,29 +216,24 @@ func (b *Service) waitForFirstBlock() error { const firstBlockTimeout = time.Minute * 5 timer := time.NewTimer(firstBlockTimeout) - cleanup := func() { - if !timer.Stop() { - <-timer.C - } - } // loop until block 1 for { select { case block, ok := <-ch: if !ok { - cleanup() + timer.Stop() return errChannelClosed } if ok && block.Header.Number > 0 { - cleanup() + timer.Stop() return nil } case <-timer.C: return errFirstBlockTimeout case <-b.ctx.Done(): - cleanup() + timer.Stop() return b.ctx.Err() } } @@ -408,28 +403,23 @@ func (b *Service) handleEpoch(epoch uint64) (next uint64, err error) { nextEpochStartTime := getSlotStartTime(nextEpochStart, b.constants.slotDuration) epochTimer := time.NewTimer(time.Until(nextEpochStartTime)) - cleanup := func() { - if !epochTimer.Stop() { - <-epochTimer.C - } - } errCh := make(chan error) go b.epochHandler.run(ctx, errCh) select { case <-b.ctx.Done(): - cleanup() + epochTimer.Stop() return 0, b.ctx.Err() case <-b.pause: - cleanup() + epochTimer.Stop() return 0, errServicePaused case <-epochTimer.C: // stop current epoch handler cancel() case err := <-errCh: // TODO: errEpochPast is sent on this channel, but it doesnot get logged here - cleanup() + epochTimer.Stop() logger.Errorf("error from epochHandler: %s", err) } diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 52e75ba3dd..885d363c2c 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -103,15 +103,6 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { logger.Debugf("start time of slot %d: %v", authoringSlot, startTime) } - defer func() { - // cleanup timers if ctx was cancelled - for _, swt := range slotTimeTimers { - if !swt.timer.Stop() { - <-swt.timer.C - } - } - }() - logger.Debugf("authoring in %d slots in epoch %d", len(slotTimeTimers), h.epochNumber) for _, swt := range slotTimeTimers { @@ -119,6 +110,9 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { select { case <-ctx.Done(): + for _, swt := range slotTimeTimers { + swt.timer.Stop() + } return case <-swt.timer.C: // we must do a time correction as the slot timer sometimes is triggered diff --git a/lib/babe/epoch_handler_test.go b/lib/babe/epoch_handler_test.go index 52b47b1117..7ff3d87883 100644 --- a/lib/babe/epoch_handler_test.go +++ b/lib/babe/epoch_handler_test.go @@ -92,9 +92,7 @@ func TestEpochHandler_run(t *testing.T) { case <-timer.C: require.Equal(t, epochLength-(firstExecutedSlot-startSlot), callsToHandleSlot) case err := <-errCh: - if !timer.Stop() { - <-timer.C - } + timer.Stop() require.NoError(t, err) } } diff --git a/lib/grandpa/finalisation.go b/lib/grandpa/finalisation.go index 07a79a273c..e6684951c5 100644 --- a/lib/grandpa/finalisation.go +++ b/lib/grandpa/finalisation.go @@ -370,14 +370,8 @@ func (f *finalisationEngine) defineRoundVotes() error { for !precommited { select { case <-f.stopCh: - if !determinePrevoteTimer.Stop() { - <-determinePrevoteTimer.C - } - - if !determinePrecommitTimer.Stop() { - <-determinePrecommitTimer.C - } - + determinePrevoteTimer.Stop() + determinePrecommitTimer.Stop() return nil case <-determinePrevoteTimer.C: @@ -389,10 +383,7 @@ func (f *finalisationEngine) defineRoundVotes() error { if alreadyCompletable { f.actionCh <- alreadyFinalized - if !determinePrecommitTimer.Stop() { - <-determinePrecommitTimer.C - } - + determinePrecommitTimer.Stop() return nil } diff --git a/tests/rpc/system_integration_test.go b/tests/rpc/system_integration_test.go index 0f413f7b0b..879ed6811e 100644 --- a/tests/rpc/system_integration_test.go +++ b/tests/rpc/system_integration_test.go @@ -50,9 +50,7 @@ func TestStableNetworkRPC(t *testing.T) { //nolint:tparallel select { case <-timer.C: case <-ctx.Done(): - if !timer.Stop() { - <-timer.C - } + timer.Stop() return } } diff --git a/tests/stress/helpers.go b/tests/stress/helpers.go index 06c86f02e1..3e54b8d8e8 100644 --- a/tests/stress/helpers.go +++ b/tests/stress/helpers.go @@ -62,9 +62,7 @@ func compareChainHeadsWithRetry(ctx context.Context, nodes node.Nodes, select { case <-timer.C: case <-ctx.Done(): - if !timer.Stop() { - <-timer.C - } + timer.Stop() return fmt.Errorf("%w: hashes=%v", err, hashes) // last error } } diff --git a/tests/stress/stress_test.go b/tests/stress/stress_test.go index 86172ec5fa..cecc86b396 100644 --- a/tests/stress/stress_test.go +++ b/tests/stress/stress_test.go @@ -553,9 +553,7 @@ func TestSync_SubmitExtrinsic(t *testing.T) { select { case <-timer.C: case <-waitNoExtCtx.Done(): - if !timer.Stop() { - <-timer.C - } + timer.Stop() require.NoError(t, waitNoExtCtx.Err()) } }