-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use vs-threading extensions where available #69932
Conversation
sharwell
commented
Sep 13, 2023
- Avoid throwing cancellation exceptions (follow-up to Avoid cancellation exceptions in async queue implementation #68412)
- Use provided NoThrowAwaitable extension method
* Avoid throwing cancellation exceptions (follow-up to dotnet#68412) * Use provided NoThrowAwaitable extension method
@@ -307,11 +307,11 @@ static async Task WaitForHighPriorityTasksAsync(CancellationToken cancellationTo | |||
if (task.IsCompleted) | |||
{ | |||
// Make sure to yield so continuations of 'task' can make progress. | |||
await Task.Yield().ConfigureAwait(false); | |||
await AwaitExtensions.ConfigureAwait(Task.Yield(), false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious why it needs to be called in extension form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➡️ Roslyn has the same extension defined in Roslyn.Utilities, and unlike how we handled NoThrowAwaitable
it wasn't renamed to ConfigureAwaitInternal
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove that though, and just use the new extension instead? (this can all come in a later PR of coursE).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➡️ Not trivially. It could be renamed, but this was the only potential conflict location. There are several references in workspaces/features where the conflict can't occur.
await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); | ||
#pragma warning disable VSTHRD004 // Await SwitchToMainThreadAsync | ||
await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable(); | ||
#pragma warning restore VSTHRD004 // Await SwitchToMainThreadAsync |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
False positive reported as microsoft/vs-threading#1232