Fix EventSource shutdown deadlock (#56453)

Fixes https://github.com/dotnet/runtime/issues/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.

diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs
index d43e49f..ce477e0 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs
@@ -7,6 +7,7 @@ using System.Collections.Generic;
 using System.Globalization;
 using System.Numerics;
 using System.Runtime.InteropServices;
+using System.Threading;
 
 #if ES_BUILD_STANDALONE
 using Microsoft.Win32;
@@ -209,6 +210,7 @@ namespace System.Diagnostics.Tracing
             // 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);
         }

diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
index 556e4e8..e47a612 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
@@ -1449,6 +1449,10 @@ namespace System.Diagnostics.Tracing
                 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
@@ -4274,16 +4278,26 @@ namespace System.Diagnostics.Tracing
 #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>