test: stabilize multi-grain cancellation - #10525
Merged
ReubenBond merged 2 commits intoAug 12, 2026
Merged
Conversation
Wait for every delayed grain call to start before cancelling the shared token so grain-side cancellation evidence cannot be skipped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Stabilizes the MultipleGrainsTaskCancellation BVT by ensuring delayed interleaving calls have actually started executing on the grain before the shared cancellation token is triggered, preventing “wait for evidence which can never exist” timeouts (notably on macOS).
Changes:
- Introduces per-grain
callIdvalues to disambiguate cancellation evidence across multiple grains. - Uses the start-notification barrier (
LongWaitInterleavingWithStartNotification+ observer) for delayed cases before scheduling cancellation. - Preserves the zero-delay path to continue exercising caller-side pre-start cancellation behavior.
Show a summary per file
| File | Description |
|---|---|
| test/Orleans.Runtime.Tests/CancellationTests/CancellationTokenTests.cs | Updates the multi-grain cancellation test to wait for grain-side start notifications and to track cancellation evidence per grain call. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Await the independent grain cancellation evidence checks concurrently so a regression remains bounded by one timeout. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 13b77a0c-9317-4fdc-b06c-cef2ea19fd63
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (1)
test/Orleans.Runtime.Tests/CancellationTests/CancellationTokenTests.cs:114
observer/observerReferenceare created and the object reference is registered even whendelay == 0, but in that branch the reference is never used (the call path usesLongWaitInterleavingrather than the...WithStartNotificationoverload). This adds unnecessary runtime work and can slightly perturb the zero-delay (pre-start cancellation) scenario. Consider creating/deleting the observer reference only whendelay > 0.
var callIds = grains.Select(_ => Guid.NewGuid()).ToArray();
var observer = new LongRunningTaskObserver();
var observerReference = fixture.GrainFactory.CreateObjectReference<ILongRunningTaskObserver>(observer);
try
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MultipleGrainsTaskCancellationcan complete caller-side cancellation before one or more delayed requests begin executing. The test then waits for grain-side cancellation evidence which can never exist, producing the five-minute timeout seen on macOS.Use the start-notification barrier added by #10461 for each delayed grain call before cancelling the shared token. Assigning each grain a distinct evidence ID lets the test prove that all five calls started and observed cancellation, while the zero-delay cases retain pre-start cancellation coverage.
Fixes #10439
Fixes #10440
Microsoft Reviewers: Open in CodeFlow