From 89f086ab36c16ecc3815e4ffba9fd112348160d3 Mon Sep 17 00:00:00 2001
From: Noah Falk <noahfalk@microsoft.com>
Date: Wed, 28 Jul 2021 02:08:19 -0700
Subject: [PATCH] Fix EventSource shutdown deadlock

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.
---
 .../Diagnostics/Tracing/EventProvider.cs       |  2 ++
 .../System/Diagnostics/Tracing/EventSource.cs  | 18 ++++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

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 d43e49f92ca3d..ce477e02c9ef7 100644
--- 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.Globalization;
 using System.Numerics;
 using System.Runtime.InteropServices;
+using System.Threading;
 
 #if ES_BUILD_STANDALONE
 using Microsoft.Win32;
@@ -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);
         }
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 556e4e8df58fe..e47a6127022cf 100644
--- 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 @@ 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
@@ -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>