Skip to content
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

Ensure IServiceBroker is used on a background thread #47903

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Sep 21, 2020

Some IServiceBroker asynchronous operations do not explicitly use ConfigureAwait(false). This change ensures we call GetProxyAsync<T> from a background thread to avoid accidentally introducing a UI thread dependency in code that should not have one.

/cc @AArnott

@sharwell sharwell requested a review from a team as a code owner September 21, 2020 19:54
Copy link
Member

@dibarbet dibarbet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some IServiceBroker asynchronous operations do not explicitly use ConfigureAwait(false)

Should they be doing ConfigureAwait(false)? If not I would be interested to know why for my own understanding. If yes, is there a bug?

@sharwell
Copy link
Member Author

Should they be doing ConfigureAwait(false)?

Probably yes, but even if that changes it wouldn't be ready for a while and still wouldn't invalidate this change.

@sharwell sharwell changed the base branch from master to release/dev16.8 September 22, 2020 00:11
@sharwell sharwell merged commit c9977cd into dotnet:release/dev16.8 Sep 22, 2020
sharwell added a commit to sharwell/roslyn that referenced this pull request Sep 28, 2020
Expands dotnet#47903 to other calls to GetProxyAsync.
@sharwell sharwell deleted the service-broker-background branch September 28, 2020 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants