Refactor async task handling to use WaitAsync for cancellation support #10425
+7
−11
Conversation
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
- This makes sure that blocking calls still yield in a timely manner
github-actions
bot
added
the
needs-area-label
There was a problem hiding this comment.
Pull Request Overview
This PR refactors DcpExecutor to make long-running Task.WhenAll calls cancellable by using WaitAsync and by offloading creation loops to Task.Run. It also removes redundant Task.Yield calls.
- Changed
Task.WhenAll(...).ConfigureAwait(false)to.WaitAsync(cancellationToken).ConfigureAwait(false)to respect cancellation. - Wrapped calls to
CreateResourceExecutablesAsyncCoreandCreateContainerAsyncCoreinTask.Runwith the cancellation token. - Removed
await Task.Yield()as it’s no longer needed.
Comments suppressed due to low confidence (1)
src/Aspire.Hosting/Dcp/DcpExecutor.cs:792
- [nitpick] No unit tests were added to verify cancellation behavior. Consider adding tests that trigger cancellation during
WaitAsyncto ensure tasks are cancelled as expected.
await Task.WhenAll(containersTask, executablesTask).WaitAsync(cancellationToken).ConfigureAwait(false);
| } | ||
|
|
||
| return Task.WhenAll(tasks); | ||
| return Task.WhenAll(tasks).WaitAsync(cancellationToken); |
There was a problem hiding this comment.
This method is declared to return Task but returns a ValueTask from WaitAsync. You should await the WaitAsync inside the async method (with .ConfigureAwait(false)) instead of returning it directly.
Suggested change
| return Task.WhenAll(tasks).WaitAsync(cancellationToken); | |
| return await Task.WhenAll(tasks).WaitAsync(cancellationToken).ConfigureAwait(false); |
Copilot uses AI. Check for mistakes.
karolz-ms
approved these changes
Jul 15, 2025
8 tasks
|
/backport to release/9.4 |
|
Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16303360659 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Description
If there's a blocking API call in the creation of the service it will block ctrl + c. This is because we assume that the cancellation token will be respected but that is not the case with these blocking APIs. This changes the logic so that WhenAll will yield if the token is fired.
Checklist