Repository navigation
test(concurrent): pin that a slot waiter abandons at its DetachTimeout - #150
Conversation
A deferred tail that waits for a MaxConcurrent slot must give up when its DetachTimeout expires, even while the slot stays held. No test checked this path: a runtime whose waiter kept waiting and then ran its handler passed the whole root suite. The new test runs in a synctest bubble. It holds the only slot, lets the waiter's DetachTimeout expire, and checks that the waiter logs the abandon, ends its dispatch span as ignored, and never runs its handler, also after the slot frees. Refs #146
|
@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 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. |
|
Codex Review: Didn't find any major issues. Nice work! 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". |
Why
The goal is to land every open follow-up from the concurrent strategy work, one small reviewed PR each. This one adds the missing test from #146: no test failed when a slot waiter kept waiting past its
DetachTimeoutand then ran its handler.What changed
TestConcurrentSlotWaiterAbandonedAtDetachTimeoutinconcurrent_test.go. It runs in atesting/synctestbubble withMaxConcurrent: 1. The first handler holds the only slot while the second delivery'sDetachTimeoutexpires.context deadline exceeded, ends its dispatch span asignored, and never runs its handler, also after the slot frees.Scope
Verification
acquireConcurrencySlotreturnstrue): the new test fails in 20 of 20 separate runs (a-count=20run stops at the first failure, because the bubble then panics). On current main this mutation also failsTestConcurrentSlotWaiterDrainedByShutdownand hangsTestConcurrentSlotFreedDuringShutdownIsAbandoneduntil thego testtimeout, because the waiter releases a slot it never took and Shutdown never drains. So the gap is narrower than the issue says.true): the new test fails, and the rest of the root suite passes. Without this PR, nothing catches that runtime.go test -race -count=50 -run 'Concurrent' .,go test -race ./..., and thevet,test:root,test:adapters,test:examplescommands frommise.tomlall pass on the head.Closes #146
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high