Repository navigation
test(concurrent): check the MaxConcurrent bound across the whole slot wait - #144
Conversation
… wait TestConcurrentBoundedByMaxConcurrent sampled the handler log once, 80 ms after the second event, then freed the slot. A slot wait that let the second handler run after that sample passed every time. The test now runs in a testing/synctest bubble. It holds the only slot on the fake clock until just before the waiter's DetachTimeout deadline. The clock moves only while every goroutine is durably blocked, so the check covers the waiter's whole wait.
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Why
The operator wants every open follow-up fixed and landed, one small reviewed PR each.
TestConcurrentBoundedByMaxConcurrentchecked the handler log once, 80 ms after the second event, then freed the slot. A slot wait that lets the second handler run after that one check passed every run. This PR makes the test check the bound for the waiter's whole wait.Follow-up from #141.
What changed
testing/synctestbubble. After it posts the second event, it sleeps on the fake clock until 1 ns before the waiter'sDetachTimeoutdeadline, then checks that only the first handler ran. The fake clock moves only while every goroutine in the bubble is durably blocked. So the check covers the full slot wait, and the test takes no wall-clock time.synctest.Waitreplaces the 2 s polling loops that checked "first handler started" and "second runs after the slot frees".Scope
TestConcurrentSlotWaiterDrainedByShutdownstays as it is. fix(runtime): abandon a concurrency slot won after Shutdown cancelled the tails #141 explains why.Blast radius
The change touches one test. If the bound check fails, a
sync.OnceFuncrelease frees the blocked handler, so the bubble reports the failure. Two limits remain. If the runtime never frees the slot, the test prints its failure and thensynctestpanics because blocked goroutines remain, which stops the package's test binary. If a goroutine blocks on async.Mutex,synctest.Waitwaits until thego testtimeout, so that failure can take up to 10 minutes to report.Verification
MaxConcurrent+1. B: the slot wait gives up after 100 ms and runs the handler anyway. The old test caught A (960 of 960 loaded runs) and missed B (0 of 200 normal runs, 0 of 960 loaded runs). With B, every otherTestConcurrent*test also passes.-cpu=1,4) and fails B in 960 of 960 loaded runs (16 CPU spinners, 24 batches of 40 at-test.cpu=1). On this head it passes 400 of 400 normal runs and 960 of 960 loaded runs.gofmt -l, andgo test -race .all pass on the head.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high