Repository navigation
Release cancellation registrations after external-event waits complete - #838
Merged
wangbill (YunchuWang) merged 3 commits intoNov 6, 2026
Merged
Conversation
Dispose registrations when their external-event tasks settle while preserving the original task, synchronous completion, waiter ordering, and canceled-event buffering. Add bounded retention controls, cancellation race coverage, and a worker package release note. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the cancellation-registration cleanup for completed external-event waits.
No open issues 🎉
What changed in this PR
This PR fixes completed external-event wait retention in the shared Durable Task worker core by releasing cancellation registrations while their token sources remain live. It preserves the original returned task, LIFO event delivery, and canceled-wait event preservation.
Changes:
- Dispose cancellation registrations on task completion without depending on the orchestrator scheduler; skip registration for noncancellable tokens and already-completed waits.
- Add eight regression cases covering task/payload retention, cancellation, synchronous continuations, ordering, and completion races.
- Document the cleanup in the worker release notes.
| File | Description |
|---|---|
test/Worker/Core.Tests/Shims/TaskOrchestrationContextWrapperTests.cs |
Adds retention and event-wait behavior regression tests. |
src/Worker/Core/Shims/TaskOrchestrationContextWrapper.cs |
Releases completed waits’ cancellation registrations and avoids unnecessary registrations. |
src/Worker/Core/RELEASENOTES.md |
Documents the cancellation-registration retention fix. |
Review effort: Balanced
Remove the collectability theory and its unused helper/import in response to review feedback. Preserve the production registration cleanup and all functional cancellation, buffered-event, synchronous/reentrant, LIFO, and race regressions unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
sophiatev
approved these changes
Nov 6, 2026
halspang
approved these changes
Nov 6, 2026
wangbill (YunchuWang)
deleted the
yunchuwang-external-event-registration-cleanup
branch
November 6, 2026 14:45
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
What changed?
ExecuteSynchronouslyandTaskScheduler.Default; skip registration for noncancellable or already-completed buffered waits.98849443d07a53c4bd17dfdd93723bc22b49ae16, remove both GC-dependent collectability cases and their unused helper/import. No allocation-pressure, timing, private-CTS inspection, or test-only production replacement was added.src/Worker/Core/RELEASENOTES.mdare unchanged by this revision. No public API, protocol, dependency, or event-dispatch changes.Why is this change needed?
WaitForExternalEvent<T>discarded itsCancellationTokenRegistration; the callback captured the completion source and could retain a completed task and payload while the token source remained live.mainretained 1,000 completed tasks with a live shared token source, versus zero in noncancellable/disposed-source controls. That GC-based reproduction is no longer in the regression suite. This is not production telemetry or evidence of cross-invocation leakage.Issues / work items
Project checklist
src/Worker/Core/RELEASENOTES.mdAI-assisted code disclosure (required)
Was an AI tool used? (select one)
If AI was used:
src/Worker/Core/Shims/TaskOrchestrationContextWrapper.cs,test/Worker/Core.Tests/Shims/TaskOrchestrationContextWrapperTests.cs, andsrc/Worker/Core/RELEASENOTES.md.AI verification (required if AI was used):
These attestations are left for a human reviewer; the automated evidence below is agent-run.
Testing
Automated tests
The retained race test performs 100 cancellation/completion races per run and passed in both wrapper configurations. A source scan found no forced-GC calls, weak-reference machinery, deleted helper/import, or timing-based substitute. Production code and release notes match the prior commit exactly. Existing unrelated analyzer warnings remain; original file endings are preserved because the repository's newline rules conflict.
The full solution, other runtimes, and live Azure/DTS deployments were not run for this revision. Remote CI and human verification are not asserted here.
Manual validation (only if runtime/behavior changed)
Notes for reviewers
CompleteExternalEventare untouched. Buffered events still win over already-canceled tokens, and canceled waiters do not consume events.main; no merge is requested.