Repository navigation
test: TestConcurrentSlotWaiterDrainedByShutdown races on a 40 ms sleep #140
Description
TestConcurrentSlotWaiterDrainedByShutdown fails intermittently because it waits on a 40 ms sleep instead of on the state it needs.
What happens
- The test fills the only slot (
MaxConcurrent = 1) with eventfirst, posts eventwaiter, then sleeps 40 ms (concurrent_test.go#L207) and callsShutdown. - It assumes
waiteris already blocked on the slot whenShutdownstarts, soShutdownmust logchat concurrent slot wait abandoned(concurrent_test.go#L220-L222). - Under load the waiter's tail starts later. In the failing CI run,
firstwas cancelled at23:40:33.078, andwaiterloggedchat deferred dispatch startedat the same millisecond, after the slot was free. The waiter never waited, so nothing logged "abandoned".
Evidence
- CI: run 40974588804, job
test (minimum)on7c6ce4a, failed atconcurrent_test.go:221. A rerun of the failed job passed. - Local, with 16 busy-loop CPU spinners and 24 parallel batches of 40 runs at
-test.cpu=1: 15 of 960 runs failed on7c6ce4a, and 6 of 960 failed on4e9fcf6. The failure predates fix(adapters/slack): drop mrkdwn from chat.postEphemeral and correct retry docs #137 and fix(runtime): free the admission slot when the deferred prelude panics #138. The two rates came from separate runs on a shared host, so they do not show that either PR changed the rate. - The test last changed in feat(runtime): deferred-dispatch admission bound per ADR 0015 #56.
Possible fix
Wait until waiter is blocked on the slot before calling Shutdown, for example through an export hook in export_test.go that reports the number of slot waiters, as QueueTurnCount does for the queue. Then remove the sleep.
Owner and trigger
The coder/chat maintainer triages this. Fix it before the next CI flake on this test, or with the next change to the concurrent strategy.
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh
Activity
ThomasK33 commentedon Oct 30, 2026
Triage:
bug,ready-for-agent. The possible fix in the issue is the plan: add a slot-waiter count hook inexport_test.go, wait on it beforeShutdown, and remove the 40 ms sleep. It already cost one CI rerun on7c6ce4a.Acceptance: run the loaded reproduction from the issue (16 spinners, 24 batches of 40 runs at
-test.cpu=1). It must show failures onmainand 0 of 960 failures with the fix. Put both counts in the PR body.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:mediumThomasK33 commentedon Oct 30, 2026
I corrected the cause in #141. The 40 ms sleep is not the problem. With instrumented runs, every failure showed the waiter had already been waiting on the slot for about 40 ms when Shutdown started.
Shutdown cancels the detached tails one at a time. When it is preempted during that pass, the first handler returns and frees its slot. The waiter then wins the slot while its own context is still live, and it runs its handler. Nothing logs "chat concurrent slot wait abandoned".
#141 makes a detached tail that wins a slot after Shutdown began release the slot and abandon the wait. It adds a test that forces the window, and it removes the sleep. On the loaded repro, main fails 3 to 13 of 960 runs and the fix fails 0 of 960, three times.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:highfix(runtime): abandon a concurrency slot won after Shutdown cancelled…
ThomasK33 commentedon Oct 30, 2026
Correction to the comment above: #141 does not remove the 40 ms sleep. A review showed that removing it made the old test reach the slot after
Shutdownin almost every run, so it rarely checked a parked waiter. The merged version keeps the sleep and fixes the runtime. With the sleep on both sides, the loaded reproduction failed 8 of 960 runs on7c6ce4aand 0 of 960 on the fix head, three times. The new testTestConcurrentSlotFreedDuringShutdownIsAbandonedcovers the race directly.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh