Fix deadlock in EventPipeEventDispatcher (dotnet#92806)

BrzVlad · Oct 2, 2023 · 9aebb66 · 9aebb66
1 parent 42a6776
commit 9aebb66
Showing 1 changed file with 20 additions and 11 deletions.
diff --git a/...braries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventDispatcher.cs b/...braries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventDispatcher.cs
@@ -103,8 +103,8 @@ private void CommitDispatchConfiguration()
                 new EventPipeProviderConfiguration(NativeRuntimeEventSource.EventSourceName, (ulong)aggregatedKeywords, (uint)enableLevel, null)
             };
 
-            m_sessionID = EventPipeInternal.Enable(null, EventPipeSerializationFormat.NetTrace, DefaultEventListenerCircularMBSize, providerConfiguration);
-            if (m_sessionID == 0)
+            ulong sessionID = EventPipeInternal.Enable(null, EventPipeSerializationFormat.NetTrace, DefaultEventListenerCircularMBSize, providerConfiguration);
+            if (sessionID == 0)
             {
                 throw new EventSourceException(SR.EventSource_CouldNotEnableEventPipe);
             }
@@ -113,7 +113,7 @@ private void CommitDispatchConfiguration()
             EventPipeSessionInfo sessionInfo;
             unsafe
             {
-                if (!EventPipeInternal.GetSessionInfo(m_sessionID, &sessionInfo))
+                if (!EventPipeInternal.GetSessionInfo(sessionID, &sessionInfo))
                 {
                     Debug.Fail("GetSessionInfo returned false.");
                 }
@@ -124,8 +124,11 @@ private void CommitDispatchConfiguration()
             long syncTimeQPC = sessionInfo.StartTimeStamp;
             long timeQPCFrequency = sessionInfo.TimeStampFrequency;
 
+            Debug.Assert(Volatile.Read(ref m_sessionID) == 0);
+            Volatile.Write(ref m_sessionID, sessionID);
+
             // Start the dispatch task.
-            StartDispatchTask(m_sessionID, syncTimeUtc, syncTimeQPC, timeQPCFrequency);
+            StartDispatchTask(sessionID, syncTimeUtc, syncTimeQPC, timeQPCFrequency);
         }
 
         private void StartDispatchTask(ulong sessionID, DateTime syncTimeUtc, long syncTimeQPC, long timeQPCFrequency)
@@ -147,10 +150,11 @@ private void SetStopDispatchTask()
                 return;
             }
 
-            Debug.Assert(m_sessionID != 0);
+            ulong sessionID = Volatile.Read(ref m_sessionID);
+            Debug.Assert(sessionID != 0);
             m_dispatchTaskCancellationSource.Cancel();
-            EventPipeInternal.SignalSession(m_sessionID);
-            m_sessionID = 0;
+            EventPipeInternal.SignalSession(sessionID);
+            Volatile.Write(ref m_sessionID, 0);
         }
 
         private unsafe void DispatchEventsToEventListeners(ulong sessionID, DateTime syncTimeUtc, long syncTimeQPC, long timeQPCFrequency, Task? previousDispatchTask, CancellationToken token)
@@ -190,12 +194,17 @@ private unsafe void DispatchEventsToEventListeners(ulong sessionID, DateTime syn
                 }
             }
 
-            lock (m_dispatchControlLock)
+            // Wait for SignalSession() to be called before we call disable, otherwise
+            // the SignalSession() call could be on a disabled session.
+            SpinWait sw = default;
+            while (Volatile.Read(ref m_sessionID) == sessionID)
             {
-                // Disable the old session. This can happen asynchronously since we aren't using the old session
-                // anymore. We take the lock to make sure we don't call SignalSession on an invalid session ID.
-                EventPipeInternal.Disable(sessionID);
+                sw.SpinOnce();
             }
+
+            // Disable the old session. This can happen asynchronously since we aren't using the old session
+            // anymore.
+            EventPipeInternal.Disable(sessionID);
         }
 
         /// <summary>