Commit 71058070 authored by Joshua Gutow's avatar Joshua Gutow Committed by GitHub

Use ticker instead of time.After in select loop (#2621)

Repeatedly using time.After in a select loop leaks resources as
resources are allocated for each timer, but not released until the
timer fires. The timer may not fire due to other return paths. By
using tickers, this resource leak is no longer able to happen.

I verified that this problem no longer exists with the following
semgrep command.
semgrep --config="r/dgryski.semgrep-go.timeafter.leaky-time-after"
Co-authored-by: default avatarmergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
parent f16383f2
...@@ -29,6 +29,8 @@ import ( ...@@ -29,6 +29,8 @@ import (
func waitForTransaction(hash common.Hash, client *ethclient.Client, timeout time.Duration) (*types.Receipt, error) { func waitForTransaction(hash common.Hash, client *ethclient.Client, timeout time.Duration) (*types.Receipt, error) {
timeoutCh := time.After(timeout) timeoutCh := time.After(timeout)
ticker := time.NewTicker(100 * time.Millisecond)
defer ticker.Stop()
ctx, cancel := context.WithTimeout(context.Background(), timeout) ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel() defer cancel()
for { for {
...@@ -42,7 +44,7 @@ func waitForTransaction(hash common.Hash, client *ethclient.Client, timeout time ...@@ -42,7 +44,7 @@ func waitForTransaction(hash common.Hash, client *ethclient.Client, timeout time
select { select {
case <-timeoutCh: case <-timeoutCh:
return nil, errors.New("timeout") return nil, errors.New("timeout")
case <-time.After(100 * time.Millisecond): case <-ticker.C:
} }
} }
} }
......
...@@ -101,10 +101,12 @@ func (s *Service) eventLoop() { ...@@ -101,10 +101,12 @@ func (s *Service) eventLoop() {
defer s.wg.Done() defer s.wg.Done()
name := s.cfg.Driver.Name() name := s.cfg.Driver.Name()
ticker := time.NewTicker(s.cfg.PollInterval)
defer ticker.Stop()
for { for {
select { select {
case <-time.After(s.cfg.PollInterval): case <-ticker.C:
// Determine the range of L2 blocks that the submitter has not // Determine the range of L2 blocks that the submitter has not
// processed, and needs to take action on. // processed, and needs to take action on.
s.l.Info(name + " fetching current block range") s.l.Info(name + " fetching current block range")
......
...@@ -205,12 +205,15 @@ func (m *SimpleTxManager) Send( ...@@ -205,12 +205,15 @@ func (m *SimpleTxManager) Send(
wg.Add(1) wg.Add(1)
go sendTxAsync() go sendTxAsync()
ticker := time.NewTicker(m.cfg.ResubmissionTimeout)
defer ticker.Stop()
for { for {
select { select {
// Whenever a resubmission timeout has elapsed, bump the gas // Whenever a resubmission timeout has elapsed, bump the gas
// price and publish a new transaction. // price and publish a new transaction.
case <-time.After(m.cfg.ResubmissionTimeout): case <-ticker.C:
// Avoid republishing if we are waiting for confirmation on an // Avoid republishing if we are waiting for confirmation on an
// existing tx. This is primarily an optimization to reduce the // existing tx. This is primarily an optimization to reduce the
// number of API calls we make, but also reduces the chances of // number of API calls we make, but also reduces the chances of
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment