Fix EventSource deadlock #71574
Merged
Fix EventSource deadlock #71574
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
Initializing NativeRuntimeEventSource inside the EventListener static constructor could lead to a deadlock where a 2nd thread holding the ETW lock calls back though ETWCallback and tries to get EventListener lock which blocks on the static constructor. Fixes dotnet#70870
stephentoub
reviewed
Jul 2, 2022
| Interlocked.CompareExchange(ref s_EventSources, new List<WeakReference<EventSource>>(2), null); | ||
| #if FEATURE_PERFTRACING | ||
| // It is possible that another thread could observe the s_EventSources list at this point and it | ||
| // won�t have the NativeRuntimeEventSource in it. In the past we guaranteed that the NativeRuntimeEventSource |
There was a problem hiding this comment.
Looks like some non-ASCII characters are trying to sneak in
stephentoub
reviewed
Jul 2, 2022
| // It is possible that another thread could observe the s_EventSources list at this point and it | ||
| // won�t have the NativeRuntimeEventSource in it. In the past we guaranteed that the NativeRuntimeEventSource | ||
| // was always visible in the list by initializing it in the EventListener static constructor, however doing it | ||
| // that way triggered deadlocks between the static constructor and an OS ETW lock. As best I can tell |
There was a problem hiding this comment.
Nit: can we avoid using "I" in such comments? After this PR who "I" is throughout the code becomes inconsistent and ambiguous.
stephentoub
reviewed
Jul 2, 2022
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
2 tasks
stephentoub
approved these changes
Jul 7, 2022
|
CI failure is known issue: #71684 |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Initializing NativeRuntimeEventSource inside the EventListener static constructor
could lead to a deadlock where a 2nd thread holding the ETW lock calls back
though ETWCallback and tries to get EventListener lock which blocks on the
static constructor.
Fixes #70870