Skip to content

Commit

Permalink
Fix EventSource shutdown deadlock (#56453)
Browse files Browse the repository at this point in the history
Fixes #48342

A deadlock was occuring because we held the EventListenersLock
while calling into EventUnregister which will take ETW's own native
lock. In the case of ETW sending an enable/disable notification
these locks are taken in reverse order which triggers a deadlock.

The fix is to ensure that we always order the locks so that any code
path taking both locks always takes the ETW lock first. In this case
it meant accumulating the list of event sources to dispose under
the lock but then exiting the lock prior to calling Dispose() which
will eventually call EventUnregister.
  • Loading branch information
noahfalk authored Jul 29, 2021
1 parent 8cf9591 commit 6482d14
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Globalization;
using System.Numerics;
using System.Runtime.InteropServices;
using System.Threading;

#if ES_BUILD_STANDALONE
using Microsoft.Win32;
Expand Down Expand Up @@ -209,6 +210,7 @@ protected virtual void Dispose(bool disposing)
// deadlocks in race conditions (dispose racing with an ETW command).
//
// We solve by Unregistering after releasing the EventListenerLock.
Debug.Assert(!Monitor.IsEntered(EventListener.EventListenersLock));
if (registrationHandle != 0)
EventUnregister(registrationHandle);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,10 @@ protected virtual void Dispose(bool disposing)
return;
}

// Do not invoke Dispose under the lock as this can lead to a deadlock.
// See https://github.com/dotnet/runtime/issues/48342 for details.
Debug.Assert(!Monitor.IsEntered(EventListener.EventListenersLock));

if (disposing)
{
#if FEATURE_MANAGED_ETW
Expand Down Expand Up @@ -4274,16 +4278,26 @@ internal static void DisposeOnShutdown()
#endif
{
Debug.Assert(EventSource.IsSupported);

List<EventSource> sourcesToDispose = new List<EventSource>();
lock (EventListenersLock)
{
Debug.Assert(s_EventSources != null);
foreach (WeakReference<EventSource> esRef in s_EventSources)
{
if (esRef.TryGetTarget(out EventSource? es))
es.Dispose();
{
sourcesToDispose.Add(es);
}
}
}

// Do not invoke Dispose under the lock as this can lead to a deadlock.
// See https://github.com/dotnet/runtime/issues/48342 for details.
Debug.Assert(!Monitor.IsEntered(EventListenersLock));
foreach (EventSource es in sourcesToDispose)
{
es.Dispose();
}
}

/// <summary>
Expand Down

0 comments on commit 6482d14

Please sign in to comment.