Repository navigation
fix(runtime): release what the prelude took when State or the logger panics - #142
Conversation
The three tests serve the runtime behind a real httptest.Server, so net/http recovers each panic. On main each one fails: a redelivery of the panicked event waits until its client gives up, the next delivery for the thread never gets the Thread Lock, and a burst member whose join log panicked never runs.
The prelude defers finishEventAcceptance, which now returns early once the entry is done. A panic from State or the logger therefore frees the entry: duplicates that wait on it fail, and a redelivery runs.
A logger panic in the queue supersede log leaves the Thread Lock with the prelude, like a MarkEvent panic. The lock test also checks that a delivery which resolves in the prelude releases its lease once.
The prelude defers releaseOnResolve, which now clears the lease it frees. Routed work takes the lease out first, so the defer frees only a lease a panic left behind. takeQueueTurn deletes the superseded waiter before it logs, so a logger panic there cannot leave that waiter pending behind a newer turn.
joinBurstBatch now starts the runner it promised in a defer. The runner still starts after the join log, and a logger panic no longer strands the member, its admission slot, and every later member of the scope.
Comment review: the deferred finish read as if every delivery ended with the panic error. abandonEventAcceptance owns the open-entry check, so finishEventAcceptance stays strict, and handLeaseToTail names the handoff. The ADR rule names locks, slots, and records instead of every resource.
|
@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. |
A failure before a gate opened left a request blocked on it, and srv.Close waited on that request until the go test timeout.
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep it up! 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 #139. When
Stateor the app'ssloghandler panics during the prelude,net/httprecovers the panic, but three things the delivery took stay taken. A redelivery of the event waits on the open acceptance entry until its own request ends. The next delivery for the thread waits forThreadLockTTL. A burst member whose join log panicked never gets a runner, so later members never run andShutdowntimes out.What changed
abandonEventAcceptanceand its ownreleaseOnResolve. The first does nothing once the prelude finished the entry. The second does nothing once the lease is released or handed to the tail. So every release still runs once, and nothing recovers the panic.joinBurstBatchstarts the burst runner in adefer. The runner still starts after the join log, and it now also starts when the log panics.takeQueueTurndeletes the superseded waiter before it logs. Without that, the released lease lets the old waiter take the lock behind the newer turn, and thecommitPendingassert ends the process underDispatchDeferred. ADR 0015 gains the rule as invariant 4.Scope
Stateand logger panics in the prelude and in the burst join, under both dispatch modes.MarkEventalready ran and the platform's retry is deduped. Under debounce the newest event is lost. IntakeQueueTurnboth the newer event and the waiter it replaced are lost. That is event loss, not a leak, and is tracked in runtime: a panicking slog handler on the supersede log loses queued events #143.main.Tradeoffs
recoverand re-panic would also free each resource. The deferred releases leave the panic value and stack untouched, as fix(runtime): free the admission slot when the deferred prelude panics #138 chose.go runBurstScopeabove the join log would also fix the burst leak, but the runner could then log before the member's join line.Blast Radius
Only the prelude, the burst join, and the queue supersede path change. On every normal path the new defers find nothing left to release.
Verification
recoverreturns 200 or 500 instead of aborting the request. A second release reportslock_release_failedor panics withclose of closed channel. A release after the handoff fails the existing lock tests, or admits a second burst member while the first holds the only slot. The lease release without thetakeQueueTurnchange trips thecommitPendingassert.-race.go vetandgo testpass in every workspace module, andgo test -racepasses for the root package.Closes #139
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh