Repository navigation
fix(runtime): free the admission slot when the deferred prelude panics - #138
Conversation
…eferred prelude Under DispatchDeferred, a State panic in the prelude runs on the net/http goroutine. The test serves the runtime behind a real httptest.Server, panics State.AcquireLock under the queue strategy and State.MarkEvent under the burst strategy, and requires that no queue turn record stays behind and that Shutdown drains before a 10s deadline. It fails on main: the panicked delivery keeps its admission slot, so Shutdown returns context deadline exceeded.
dispatchDeferred freed the slot only when the prelude returned. A State panic unwound past it to net/http, which recovers the panic, so the delivery kept its slot and its queue turn record, and Shutdown timed out. A deferred call now runs the release on every exit that hands no work to a tail, a panic included. The panic still propagates to net/http. The release is the sync.OnceFunc from #134, so no exit can release twice.
|
@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. Bravo. 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
Every open follow-up issue in coder/chat gets one small, reviewed PR, and this one closes #135. Under
DispatchDeferred, aStatepanic in the prelude unwinds tonet/http, which recovers it.dispatchDeferredfreed the admission slot only when the prelude returned, so the panicked delivery kept its slot and its queue turn record, andShutdowntimed out.What changed
dispatchDeferredfrees the slot in adeferon every exit that hands no work to a tail, a panic included. The panic still reachesnet/http.sync.OnceFuncfrom fix(runtime): harden the queue turn record against a double release and a State panic #134, so no exit can release twice.httptest.Server. It panicsState.AcquireLockunder the queue strategy andState.MarkEventunder the burst strategy.Scope
dispatchDeferred. Before the fix, aMarkEventpanic leaked the slot under drop, debounce, burst, and concurrent too.main.Tradeoffs
recoverand re-panic would also free the slot. Thedeferwith an ownership flag frees it and leaves the panic value and stack untouched.Blast Radius
Only deferred dispatch changes. A delivery that resolves in the prelude now frees its slot as
dispatchDeferredreturns, not just before, which the caller cannot tell apart.Verification
Shutdown ... context deadline exceeded. The queue case also keeps 1 turn record.recoverfails the new test with status 200. A release withoutOnceFunctrips theleaveQueueScopeassert in the queue tests. A release without the ownership flag fails two admission tests.-race.go vetandgo testpass in every workspace module, andgo test -racepasses for the root package.Closes #135
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh