From 24367aadfbcc3c8788d9bb542fee4884d85e7022 Mon Sep 17 00:00:00 2001
From: Mitchell Hwang <16830051+mdh1418@users.noreply.github.com>
Date: Fri, 26 Jul 2024 17:37:24 -0400
Subject: [PATCH] [EventSource] Block EventCommandExecuted callback during
 initialization (#105341)

* [EventSource] Relax assert on CounterGroup callback

* [EventSource] Avoid callback double invoke

Guard EventCommandExecuted deferred commands callbacks with
EventSource initialization to prevent CounterGroup's callback
registration from seeing unexpected commands

* Simplify comments for processing deferred commands
---
 .../System/Diagnostics/Tracing/EventSource.cs | 26 ++++++++++++-------
 1 file changed, 16 insertions(+), 10 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
index 7d5365d8d2481..b4fa216916878 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
@@ -500,13 +500,16 @@ public event EventHandler<EventCommandEventArgs>? EventCommandExecuted
 
                 m_eventCommandExecuted += value;
 
-                // If we have an EventHandler<EventCommandEventArgs> attached to the EventSource before the first command arrives
-                // It should get a chance to handle the deferred commands.
-                EventCommandEventArgs? deferredCommands = m_deferredCommands;
-                while (deferredCommands != null)
+                if (m_completelyInited)
                 {
-                    value(this, deferredCommands);
-                    deferredCommands = deferredCommands.nextCommand;
+                    // If we have an EventHandler<EventCommandEventArgs> attached to the EventSource before the first command arrives
+                    // It should get a chance to handle the deferred commands.
+                    EventCommandEventArgs? deferredCommands = m_deferredCommands;
+                    while (deferredCommands != null)
+                    {
+                        value(this, deferredCommands);
+                        deferredCommands = deferredCommands.nextCommand;
+                    }
                 }
             }
             remove
@@ -1638,8 +1641,6 @@ private unsafe void Initialize(Guid eventSourceGuid, string eventSourceName, str
                 m_eventPipeProvider = eventPipeProvider;
 #endif
                 Debug.Assert(!m_eventSourceEnabled);     // We can't be enabled until we are completely initted.
-                // We are logically completely initialized at this point.
-                m_completelyInited = true;
             }
             catch (Exception e)
             {
@@ -1660,6 +1661,11 @@ private unsafe void Initialize(Guid eventSourceGuid, string eventSourceName, str
                     DoCommand(deferredCommands);      // This can never throw, it catches them and reports the errors.
                     deferredCommands = deferredCommands.nextCommand;
                 }
+
+                if (m_constructionException == null)
+                {
+                    m_completelyInited = true;
+                }
             }
         }
 
@@ -2573,9 +2579,9 @@ internal void DoCommand(EventCommandEventArgs commandArgs)
             }
 
             // PRECONDITION: We should be holding the EventListener.EventListenersLock
-            // We defer commands until we are completely inited.  This allows error messages to be sent.
-            Debug.Assert(m_completelyInited);
+            Debug.Assert(Monitor.IsEntered(EventListener.EventListenersLock));
 
+            // We defer commands until we can send error messages.
             if (m_etwProvider == null)     // If we failed to construct
                 return;