syncclient: leaked Run goroutine races the computeCapacity test hook under -shuffle #93

New issue

Open

opened 2026-08-22 16:03:23 +00:00 by a73x · 1 comment

a73x commented

2026-08-22 16:03:23 +00:00

(Migrated from git.a73x.sh)

[claude 2026-08-22] make ci failed on a data race in internal/agent/syncclient. Pre-existing and order-dependent — it has nothing to do with the branch that hit it (a Makefile-only change), and make ci is green on a luckier shuffle.

WARNING: DATA RACE
Write at 0x0000020d28d0 by goroutine 162:
  syncclient.TestAdvertisedCapacityReprobesUntilSuccess()  client_test.go:86
Previous read at 0x0000020d28d0 by goroutine 149:
  syncclient.(*Client).advertisedCapacity()  client.go:166
  syncclient.(*Client).session()             client.go:344
  syncclient.(*Client).Run()                 client.go:250
  syncclient.TestAuthRejectedClassified.gowrap1()  client_test.go:336

Two defects meeting:

TestAuthRejectedClassified (client_test.go:336) does go dc.Run(rctx), then rcancel() and returns WITHOUT waiting for Run to exit. The goroutine outlives the test and keeps calling advertisedCapacity.
TestAdvertisedCapacityReprobesUntilSuccess (client_test.go:83) swaps the package-level computeCapacity func var. Nothing synchronises that write against the leaked reader.

So it only fires when -shuffle puts AuthRejected before AdvertisedCapacity AND the leaked goroutine is still in-flight.

Repro (about 2 runs in 3):

go test -race -shuffle=1787414483164250487 -count=1 ./internal/agent/syncclient

Fix belongs on the leak: make Run's goroutine joinable and wait for it before the test returns (a done channel closed by the goroutine). That removes the class, not just this pair — any test that swaps a package-level hook is exposed to any test that leaks a client.

Related: the tree-wide goroutine-leak cleanup already done for apiServer.

Migrated from git-collab issue a0a18e7fa18955327a5d86611ab92c52703479c6.

Original author: a73x dev@a73x.sh.

[claude 2026-08-22] `make ci` failed on a data race in internal/agent/syncclient. Pre-existing and order-dependent — it has nothing to do with the branch that hit it (a Makefile-only change), and `make ci` is green on a luckier shuffle. WARNING: DATA RACE Write at 0x0000020d28d0 by goroutine 162: syncclient.TestAdvertisedCapacityReprobesUntilSuccess() client_test.go:86 Previous read at 0x0000020d28d0 by goroutine 149: syncclient.(*Client).advertisedCapacity() client.go:166 syncclient.(*Client).session() client.go:344 syncclient.(*Client).Run() client.go:250 syncclient.TestAuthRejectedClassified.gowrap1() client_test.go:336 Two defects meeting: 1. TestAuthRejectedClassified (client_test.go:336) does `go dc.Run(rctx)`, then `rcancel()` and returns WITHOUT waiting for Run to exit. The goroutine outlives the test and keeps calling advertisedCapacity. 2. TestAdvertisedCapacityReprobesUntilSuccess (client_test.go:83) swaps the package-level `computeCapacity` func var. Nothing synchronises that write against the leaked reader. So it only fires when -shuffle puts AuthRejected before AdvertisedCapacity AND the leaked goroutine is still in-flight. Repro (about 2 runs in 3): go test -race -shuffle=1787414483164250487 -count=1 ./internal/agent/syncclient Fix belongs on the leak: make Run's goroutine joinable and wait for it before the test returns (a done channel closed by the goroutine). That removes the class, not just this pair — any test that swaps a package-level hook is exposed to any test that leaks a client. Related: the tree-wide goroutine-leak cleanup already done for apiServer. --- Migrated from git-collab issue `a0a18e7fa18955327a5d86611ab92c52703479c6`. Original author: a73x <dev@a73x.sh>.

a73x commented

2026-08-23 05:38:15 +00:00

(Migrated from git.a73x.sh)

[claude 2026-08-23] Sighted again on 2026-08-23 under make test (-race -shuffle=on) while working the 5e59e2b followups, seed 1787463264684504504:

Write at 0x0000020d28d0 by goroutine 557:
  syncclient.TestAdvertisedCapacityReprobesUntilSuccess() client_test.go:86
Previous read at 0x0000020d28d0 by goroutine 532:
  syncclient.(*Client).advertisedCapacity() client.go:166
  syncclient.(*Client).session() client.go:344

Not reproducible from the seed: re-running the whole suite with that exact seed, on both the branch and main, was clean, and the package alone with that seed is clean. So it is load/timing, not ordering — which rules out fixing it by pinning a shuffle order.

[claude 2026-08-23] Sighted again on 2026-08-23 under `make test` (-race -shuffle=on) while working the 5e59e2b followups, seed 1787463264684504504: Write at 0x0000020d28d0 by goroutine 557: syncclient.TestAdvertisedCapacityReprobesUntilSuccess() client_test.go:86 Previous read at 0x0000020d28d0 by goroutine 532: syncclient.(*Client).advertisedCapacity() client.go:166 syncclient.(*Client).session() client.go:344 Not reproducible from the seed: re-running the whole suite with that exact seed, on both the branch and main, was clean, and the package alone with that seed is clean. So it is load/timing, not ordering — which rules out fixing it by pinning a shuffle order.