Repository navigation
fix(runtime): supersede an older queue waiter when a newer event takes the free lock - #122
Conversation
A newer delivery that finds the Thread Lock free takes it in its prelude. The older delivery that was parked for the lock still runs afterwards, so the handler sees A, C, B. The test holds B's first lock poll with a gated State, lets C run, then releases B. It fails under both dispatch modes. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
…s the free lock Under ConcurrencyQueue, a routed event that takes the free Thread Lock in its prelude now supersedes the scope's pending waiter when that waiter was admitted earlier. The waiter stops at its next poll or at its re-check after AcquireLock, ends as ignored, and the log records superseded_by. Before, the waiter still ran after the newer event, so the bot answered an older message last. The call sits after acceptEvent, so an event that resolves in its prelude (a duplicate, an ignored or unrouted event) supersedes no one. A waiter admitted after the lock holder stays and runs next. Two guard tests pin both rules. registerPending and the new path share displaceLocked, which writes the skip record. Closes #121 --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4568acd90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
A check that fails before the test opens a gate left B's lock poll or A's handler blocked, so the unbounded Shutdown in cleanup waited forever. The test hit the binary timeout and its own failure message never printed. A 5s shutdown context lets the failure report. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Swish! 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. |
Readiness recordDecision: ready to merge, with one tracked follow-up (#123). Not merged: the owner merges.
Generated with |
Why
Under
ConcurrencyQueue, the bot must never answer an older message after a newer one (#121). A newer event that found the Thread Lock free took it in its prelude and ran. The older event that waited for the lock still ran afterwards, so the bot answered the old message last.What changed
routedWork: a queue event that took the free lock calls the newsupersedeOlderWaiter. UnderqueueMu, it supersedes and deletes a pending waiter that was admitted earlier, and keeps a newer one. The waiter ends as ignored, and the log recordssuperseded_by.registerPendingandsupersedeOlderWaitersharedisplaceLocked, which writes the skip record and closes the waiter's displacement channel.ConcurrencyQueueGoDoc states the rule.Scope
mainhas the same gap.Tradeoffs
AcquireLockwas rejected. A TLA+ model of this coordination found a path where an older delivery registers after that check and still runs after the lock taker.AcquireLock. So a duplicate or unrouted event that takes the lock never knocks out the waiter.Blast Radius
Only the queue strategy changes, and only when an event finds the lock free while an older event waits. That older event is now skipped as superseded, which is what the queue contract promises.
Verification
TestQueueNewerDeliveryThatTakesTheFreeLockSupersedesParkedWaiterlands first and fails withhandled = [A C B]in deferred and sync dispatch. With the fix it passes 5 of 5 runs under-race.AcquireLock(handled = [A]) or loses its sequence check (handled = [A C]).go test -race ./...andgo vetin every workspace module pass. The TLA+ model of the fixed code (not in this repo yet) passes every invariant with up to 4 deliveries.Closes #121
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh