Skip to content

Commit d1e6588

Browse files
xiamCopilot
andauthored
ethmonitor: add poc for goroutine leak during shutdown (#197)
* ethmonitor: add poc for goroutine leak during shutdown * ethmonitor: fixes after code review * ethmonitor: fix goroutine leak * rename PoC test to TestMonitorShutdownNoGoroutineLeak Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * ethmonitor: fix goroutine leak --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 2356e6f commit d1e6588

5 files changed

Lines changed: 815 additions & 5 deletions

File tree

‎ethmonitor/ethmonitor.go‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,10 @@ func (m *Monitor) listenNewHead() <-chan uint64 {
441441
return
442442

443443
case <-time.After(time.Duration(m.pollInterval.Load())):
444-
nextBlock <- 0
444+
select {
445+
case nextBlock <- 0:
446+
case <-m.ctx.Done():
447+
}
445448
}
446449
}
447450
}
@@ -474,12 +477,24 @@ func (m *Monitor) listenNewHead() <-chan uint64 {
474477
if nextBlockNumber == 0 || latestBlockNum > nextBlockNumber {
475478
// monitor is behind, so we just push to keep going without
476479
// waiting on the nextBlock channel
477-
ch <- nextBlockNumber
480+
select {
481+
case ch <- nextBlockNumber:
482+
case <-m.ctx.Done():
483+
return
484+
}
478485
continue
479486
} else {
480487
// wait for the next block
481-
<-nextBlock
482-
ch <- latestBlockNum
488+
select {
489+
case <-nextBlock:
490+
case <-m.ctx.Done():
491+
return
492+
}
493+
select {
494+
case ch <- latestBlockNum:
495+
case <-m.ctx.Done():
496+
return
497+
}
483498
}
484499
}
485500
}()
@@ -1008,7 +1023,10 @@ func (m *Monitor) publish(ctx context.Context, events Blocks) error {
10081023
// Publish events existing in the queue
10091024
pubEvents, ok := m.publishQueue.dequeue(maxBlockNum)
10101025
if ok {
1011-
m.publishCh <- pubEvents
1026+
select {
1027+
case m.publishCh <- pubEvents:
1028+
case <-m.ctx.Done():
1029+
}
10121030
}
10131031

10141032
return nil
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package ethmonitor_test
2+
3+
//go:generate mockgen -destination=internal/mocks/mock_provider.go -package=mocks github.com/0xsequence/ethkit/ethrpc RawInterface
4+
5+
import (
6+
"context"
7+
"fmt"
8+
"math/big"
9+
"runtime"
10+
"testing"
11+
"time"
12+
13+
"github.com/0xsequence/ethkit/ethmonitor"
14+
"github.com/0xsequence/ethkit/ethmonitor/internal/mocks"
15+
"go.uber.org/mock/gomock"
16+
)
17+
18+
// TestMonitorShutdownNoGoroutineLeak verifies that the monitor shuts down cleanly without leaking goroutines.
19+
func TestMonitorShutdownNoGoroutineLeak(t *testing.T) {
20+
if testing.Short() {
21+
t.Skip("Skipping in short mode")
22+
}
23+
24+
ctrl := gomock.NewController(t)
25+
defer ctrl.Finish()
26+
27+
provider := mocks.NewMockRawInterface(ctrl)
28+
provider.EXPECT().ChainID(gomock.Any()).Return(big.NewInt(1), nil).AnyTimes()
29+
provider.EXPECT().IsStreamingEnabled().Return(false).AnyTimes()
30+
provider.EXPECT().RawBlockByNumber(gomock.Any(), gomock.Any()).
31+
Return(nil, fmt.Errorf("simulated network error")).AnyTimes()
32+
33+
baseline := runtime.NumGoroutine()
34+
35+
opts := ethmonitor.DefaultOptions
36+
opts.PollingInterval = 10 * time.Millisecond
37+
opts.Timeout = 50 * time.Millisecond
38+
39+
monitor, err := ethmonitor.NewMonitor(provider, opts)
40+
if err != nil {
41+
t.Fatalf("failed to create monitor: %v", err)
42+
}
43+
44+
ctx, cancel := context.WithCancel(context.Background())
45+
46+
done := make(chan error, 1)
47+
go func() {
48+
done <- monitor.Run(ctx)
49+
}()
50+
51+
time.Sleep(200 * time.Millisecond)
52+
cancel()
53+
54+
select {
55+
case <-done:
56+
case <-time.After(5 * time.Second):
57+
t.Fatal("Monitor.Run() didn't return within timeout")
58+
}
59+
60+
runtime.GC()
61+
time.Sleep(100 * time.Millisecond)
62+
63+
if leaked := runtime.NumGoroutine() - baseline; leaked > 0 {
64+
t.Fatalf("%d goroutine(s) leaked", leaked)
65+
}
66+
}

0 commit comments

Comments
 (0)