Skip to content

Commit 5b57739

Browse files
ernadoclaude
andcommitted
test(pool): make kill flags atomic to fix RunGC data race
The GC goroutine cancels a managed bot while the test asserts on the flag; use atomic.Bool so -race is clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ff3a162 commit 5b57739

1 file changed

Lines changed: 16 additions & 14 deletions

File tree

pool/pool_internal_test.go

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,17 @@ package pool
33
import (
44
"context"
55
"errors"
6+
"sync/atomic"
67
"testing"
78
"time"
89
)
910

1011
// fakeManaged returns a managed entry that is already "ready", with a cancel
11-
// hook recording whether it was killed.
12-
func fakeManaged(killed *bool) *managed {
12+
// hook recording whether it was killed. The flag is atomic because the pool may
13+
// cancel from a GC goroutine.
14+
func fakeManaged(killed *atomic.Bool) *managed {
1315
m := &managed{ready: make(chan struct{})}
14-
m.cancel = func() { *killed = true }
16+
m.cancel = func() { killed.Store(true) }
1517
m.markReady(nil)
1618
return m
1719
}
@@ -48,11 +50,11 @@ func TestManagedIdleBefore(t *testing.T) {
4850

4951
func TestKillRemovesAndCancels(t *testing.T) {
5052
p, _ := New(Options{AppID: 1, AppHash: "x"})
51-
killed := false
53+
var killed atomic.Bool
5254
p.bots["123:abc"] = fakeManaged(&killed)
5355

5456
p.Kill("123:abc")
55-
if !killed {
57+
if !killed.Load() {
5658
t.Fatal("Kill should cancel the bot")
5759
}
5860
if _, ok := p.bots["123:abc"]; ok {
@@ -64,13 +66,13 @@ func TestKillRemovesAndCancels(t *testing.T) {
6466

6567
func TestCloseKillsAll(t *testing.T) {
6668
p, _ := New(Options{AppID: 1, AppHash: "x"})
67-
k1, k2 := false, false
69+
var k1, k2 atomic.Bool
6870
p.bots["1:a"] = fakeManaged(&k1)
6971
p.bots["2:b"] = fakeManaged(&k2)
7072

7173
p.Close()
72-
if !k1 || !k2 {
73-
t.Fatalf("Close should kill all: %v %v", k1, k2)
74+
if !k1.Load() || !k2.Load() {
75+
t.Fatalf("Close should kill all: %v %v", k1.Load(), k2.Load())
7476
}
7577
if len(p.bots) != 0 {
7678
t.Fatalf("pool not emptied: %d", len(p.bots))
@@ -79,7 +81,7 @@ func TestCloseKillsAll(t *testing.T) {
7981

8082
func TestReapCollectsIdle(t *testing.T) {
8183
p, _ := New(Options{AppID: 1, AppHash: "x"})
82-
idleKilled, freshKilled := false, false
84+
var idleKilled, freshKilled atomic.Bool
8385

8486
idle := fakeManaged(&idleKilled)
8587
idle.lastUsed = time.Now().Add(-time.Hour)
@@ -91,10 +93,10 @@ func TestReapCollectsIdle(t *testing.T) {
9193

9294
p.reap(time.Now().Add(-time.Minute))
9395

94-
if !idleKilled {
96+
if !idleKilled.Load() {
9597
t.Fatal("idle bot should be reaped")
9698
}
97-
if freshKilled {
99+
if freshKilled.Load() {
98100
t.Fatal("fresh bot should survive")
99101
}
100102
if _, ok := p.bots["idle"]; ok {
@@ -107,7 +109,7 @@ func TestReapCollectsIdle(t *testing.T) {
107109

108110
func TestRunGCReapsThenStops(t *testing.T) {
109111
p, _ := New(Options{AppID: 1, AppHash: "x", IdleTimeout: 10 * time.Millisecond})
110-
killed := false
112+
var killed atomic.Bool
111113
m := fakeManaged(&killed)
112114
m.lastUsed = time.Now().Add(-time.Hour)
113115
p.bots["idle"] = m
@@ -131,7 +133,7 @@ func TestRunGCReapsThenStops(t *testing.T) {
131133
case <-time.After(5 * time.Millisecond):
132134
}
133135
}
134-
if !killed {
136+
if !killed.Load() {
135137
t.Fatal("reaped bot should be killed")
136138
}
137139

@@ -145,7 +147,7 @@ func TestRunGCReapsThenStops(t *testing.T) {
145147

146148
func TestAcquireDedupes(t *testing.T) {
147149
p, _ := New(Options{AppID: 1, AppHash: "x"})
148-
killed := false
150+
var killed atomic.Bool
149151
existing := fakeManaged(&killed)
150152
p.bots["123:abc"] = existing
151153

0 commit comments

Comments
 (0)