don't call validate while messing with eventlistener list, and move c…

…all to disableevents out of the main loop Update EventSource.cs
dotnet · Mar 21, 2023 · c945d04 · c945d04
1 parent 84654a6
commit c945d04
Showing 1 changed file with 44 additions and 9 deletions.
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
@@ -4222,18 +4222,33 @@ internal static void DisposeOnShutdown()
         // If an EventListener calls Dispose without calling DisableEvents first we want to issue the Disable command now
         private static void CallDisableEventsIfNecessary(EventDispatcher eventDispatcher, EventSource eventSource)
         {
-            if (eventDispatcher.m_EventEnabled == null)
+#if DEBUG
+            // Disable validation of EventSource/EventListener connections in case a call to EventSource.AddListener
+            // causes a recursive call into this method.
+            bool previousValue = s_ConnectingEventSourcesAndListener;
+            s_ConnectingEventSourcesAndListener = true;
+            try
             {
-                return;
-            }
+#endif
+                if (eventDispatcher.m_EventEnabled == null)
+                {
+                    return;
+                }
 
-            for (int i = 0; i < eventDispatcher.m_EventEnabled.Length; ++i)
-            {
-                if (eventDispatcher.m_EventEnabled[i])
+                for (int i = 0; i < eventDispatcher.m_EventEnabled.Length; ++i)
                 {
-                    eventDispatcher.m_Listener.DisableEvents(eventSource);
+                    if (eventDispatcher.m_EventEnabled[i])
+                    {
+                        eventDispatcher.m_Listener.DisableEvents(eventSource);
+                    }
                 }
+#if DEBUG
+            }
+            finally
+            {
+                s_ConnectingEventSourcesAndListener = previousValue;
             }
+#endif
         }
 
         /// <summary>
@@ -4247,6 +4262,27 @@ private static void RemoveReferencesToListenerInEventSources(EventListener liste
             Debug.Assert(Monitor.IsEntered(EventListener.EventListenersLock));
             // Foreach existing EventSource in the appdomain
             Debug.Assert(s_EventSources != null);
+
+            // First pass to call DisableEvents
+            foreach (WeakReference<EventSource> eventSourceRef in s_EventSources)
+            {
+                if (eventSourceRef.TryGetTarget(out EventSource? eventSource))
+                {
+                    EventDispatcher? cur = eventSource.m_Dispatchers;
+                    while (cur != null)
+                    {
+                        if (cur.m_Listener == listenerToRemove)
+                        {
+                            CallDisableEventsIfNecessary(cur!, eventSource);
+                        }
+
+                        cur = cur.m_Next;
+                    }
+                }
+            }
+
+            // DisableEvents can call back to user code and we have to start over since s_EventSources and
+            // eventSource.m_Dispatchers could have mutated
             foreach (WeakReference<EventSource> eventSourceRef in s_EventSources)
             {
                 if (eventSourceRef.TryGetTarget(out EventSource? eventSource))
@@ -4255,7 +4291,6 @@ private static void RemoveReferencesToListenerInEventSources(EventListener liste
                     // Is the first output dispatcher the dispatcher we are removing?
                     if (eventSource.m_Dispatchers.m_Listener == listenerToRemove)
                     {
-                        CallDisableEventsIfNecessary(eventSource.m_Dispatchers!, eventSource);
                         eventSource.m_Dispatchers = eventSource.m_Dispatchers.m_Next;
                     }
                     else
@@ -4272,7 +4307,6 @@ private static void RemoveReferencesToListenerInEventSources(EventListener liste
                             }
                             if (cur.m_Listener == listenerToRemove)
                             {
-                                CallDisableEventsIfNecessary(cur!, eventSource);
                                 prev.m_Next = cur.m_Next;       // Remove entry.
                                 break;
                             }
@@ -4288,6 +4322,7 @@ private static void RemoveReferencesToListenerInEventSources(EventListener liste
 #endif // FEATURE_PERFTRACING
         }
 
+
         /// <summary>
         /// Checks internal consistency of EventSources/Listeners.
         /// </summary>