feat(actor)!: let IProtocolActor.PostAsync be withdrawn with a CancellationToken - #242
Conversation
…lationToken PostAsync(Action) and PostAsync<T>(Func<T>) had no way to give up on work that was still waiting in the mailbox. Both now take an optional CancellationToken: a token cancelled before the call, or while the item is queued, cancels the returned task at once and the work never runs. Work that has started is never interrupted and its task reports the result, so the single-writer discipline (FR-RAW-021) never sees a half-finished item. The queued-to-started transition is one atomic step that the loop and the token race for, so work that started is never reported as cancelled and work that was reported as cancelled never runs. The ADR window before 1.3.0 is the last point where an interface member can be changed without a major version, so the signatures are replaced rather than overloaded next to the old ones. BREAKING CHANGE: IProtocolActor.PostAsync and PostAsync<T> gained a CancellationToken parameter. Callers compile unchanged; implementers of IProtocolActor and code compiled against 1.2.x must be rebuilt. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
PR SummaryMedium Risk Overview Callers can withdraw mailbox work while it is still queued: cancellation before the call or while waiting in the mailbox completes the returned task as canceled and never runs the delegate. Once the loop has started the item, cancellation is ignored—the work finishes and the task reports success or failure, preserving FR-RAW-021 single-writer semantics.
Docs ( Reviewed by Cursor Bugbot for commit 78b4cd8. Bugbot is set up for automated code reviews on this repo. Configure here. |
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. |
![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: 6516b28c93
ℹ️ 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".
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6516b28. Configure here.
PostAsync registers with the caller's token before it enqueues. When the actor is disposed first, PostInternal throws before anything is enqueued, so no mailbox wrapper ever runs to dispose that registration and no task is returned to hold it. A long-lived token kept every such call alive until it was cancelled or disposed. The enqueue is now wrapped once, in a helper both overloads share, and disposes the registration when it throws. The dispatch-failure handling moves into the same helper so it exists once instead of twice. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
When the token won the queued-to-started race, the mailbox wrapper returned before the try/finally that disposes the call, so the registration on the token was never released on that path. The wrapper now owns the call with a using statement, which covers the withdrawn path as well as the completed and faulted ones. The registration field becomes readonly, assigned once in the constructor, which is what the analyser asked for and what the type always intended. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9639e7a9e
ℹ️ 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".
In SynchronizationContext mode a failing Send completes the task from the dispatch-failure handler. That handler bypassed the queued-to-started transition the loop and the token race for, so a token that had already won it and was about to cancel the task could be overtaken by the fault: work withdrawn before it started would surface as faulted instead of cancelled. The handler now goes through the same transition and faults the task only when the token has not withdrawn the item first. Two tests cover the neighbouring paths that had none: PostAsync on a disposed actor with a token still throws synchronously instead of returning a task that never completes, and a failing Send with a live token faults the task. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
3 participants
What does this change?
IProtocolActor.PostAsync(Action)andPostAsync<T>(Func<T>)had no way to give up on work still waiting in the mailbox. Both now take an optionalCancellationToken:The "queued → started" transition is a single atomic step that the loop and the token race for, so work that started is never reported as cancelled and work that was reported as cancelled never runs.
Found while going through checklist item 4 of ADR 0001 (API baselines reviewed as a whole before the freeze), together with #238. The window before the
v1.3.0tag is the last point where an interface member can change without a major version, so the signatures are replaced, not overloaded next to the old ones and not shadowed with[Obsolete].The three
IProtocolActortest doubles (DeadlineTests,BusStateMonitorTests,HeartbeatProducerTests) follow the new signature. Internal callers need no change.Type of change
feat— new behaviour (minor release)fix/perf— bug or performance fix (patch release)docs/test/refactor/chore/ci— no release!in the title, plus aBREAKING CHANGE:footer explaining the migration)The commit is a
featwith!and aBREAKING CHANGE:footer in its message. While the ADR window is open,breakingmaps tominorin.releaserc.json.Checklist
dotnet build CanKit.Pro.sln -c Releasesucceeds (run with-p:CI=true, 0 warnings)dotnet test CanKit.Pro.sln -c Releasepasses (run with--framework net10.0: 1380 passed; thenet48leg was not run locally)ProtocolActorTests)Also run locally:
dotnet format --verify-no-changes(clean) anddotnet packwitheng/verify-packages.py(9 packages). The Actor approval baseline was taken from the generated.received.txt; its diff is exactly the four changed signature lines.Mutation check
Each of these breaks the implementation and makes at least one of the new tests fail:
TryStartalways succeeds): fails the "cancelled while queued" and the "cancelled after start" testsOne thing the tests do not pin: the early return for a token that is already cancelled on entry. Removing it leaves the suite green, because the registration on an already-cancelled token fires at once and the work is skipped anyway. It is an optimisation, so the XML documentation does not promise that nothing is enqueued.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
Generated by Claude Code