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

open   by a73x

Labels: backlog

[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.

Comments

[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.