[6.0.2] Call sync connection close instead of async #26943
Merged
Conversation
This file contains 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
roji
commented
Dec 8, 2021
src/EFCore/Metadata/Conventions/ForeignKeyAttributeConvention.cs
Outdated
Show resolved
Hide resolved
roji
changed the title
AndriySvyryd
approved these changes
Dec 8, 2021
Also add missing ConfigureAwait(false) and StringComparison.Ordinal, because of a configuration mismatch between FxCop and the built-in diagnostics analyzers. Fixes dotnet#26790
ajcvickers
approved these changes
Dec 13, 2021
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.
Fixes #26790, replaces #26793
Description
As part of logging path optimizations in 6.0, an (unawaited) async connection Close method call was accidentally introduced in a synchronous code path. In addition, while investigating this some cases of missing ConfigureAwait(false) were discovered.
Customer impact
In some providers, where pooling isn't used, the unawaited async call causes a race condition with unknown effects, potentially crashing the application. The cases of missing ConfigureAwait(false) could potentially cause EF Core code to run in the user's synchronization context instead of on the thread pool.
How found
A customer reported the unawaited async call on 6.0. The other two issues were found as a result of fixing an analyzer issue which suppressed warnings.
Regression
Yes, from 5.0.
Testing
Verifying that an async method is called instead of a sync one isn't feasible.
Risk
Very low - an obvious one line fix to an obvious copy-paste mistake for the unawaited async call, and trivial additions of
ConfigureAwait(false)where missing.