PR feedback

dotnet · Aug 8, 2024 · 05595e6 · 05595e6
1 parent 85ff378
commit 05595e6
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 14 deletions.
diff --git a/...raries/System.Diagnostics.DiagnosticSource/src/ILLink/ILLink.Descriptors.LibraryBuild.xml b/...raries/System.Diagnostics.DiagnosticSource/src/ILLink/ILLink.Descriptors.LibraryBuild.xml
@@ -1,7 +1,7 @@
 <linker>
   <assembly fullname="System.Diagnostics.DiagnosticSource">
     <type fullname="System.Diagnostics.Metrics.MetricsEventSource">
-      <!-- Used by S.P.C via reflection to init the EventSource -->
+      <!-- Used by System.Private.CoreLib via reflection to init the EventSource -->
       <method name="GetInstance" />
     </type>
   </assembly>

diff --git a/...raries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/...raries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
@@ -25,6 +25,7 @@ System.Diagnostics.DiagnosticSource</PackageDescription>
 
   <ItemGroup>
     <ILLinkSubstitutionsXmls Include="ILLink/ILLink.Substitutions.Shared.xml" />
+    <Content Include="ILLink\ILLink.Descriptors.LibraryBuild.xml" />
   </ItemGroup>
 
   <ItemGroup>
@@ -85,7 +86,6 @@ System.Diagnostics.DiagnosticSource</PackageDescription>
     <Compile Include="$(CommonPath)Internal\Padding.cs" Link="Common\Internal\Padding.cs" />
     <Compile Include="$(CommonPath)System\HexConverter.cs" Link="Common\System\HexConverter.cs" />
     <Compile Include="$(CommonPath)System\Diagnostics\DiagnosticsHelper.cs" Link="Common\System\Diagnostics\DiagnosticsHelper.cs" />
-    <Content Include="ILLink\ILLink.Descriptors.LibraryBuild.xml" />
 
     <None Include="DiagnosticSourceUsersGuide.md" />
     <None Include="ActivityUserGuide.md" />

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
@@ -2407,19 +2407,19 @@ private sealed class OverrideEventProvider : EventProvider
             public OverrideEventProvider(Func<EventSource?> eventSourceFactory, EventProviderType providerType)
                 : base(providerType)
             {
-                this.m_eventSourceFactory = eventSourceFactory;
-                this.m_eventProviderType = providerType;
+                _eventSourceFactory = eventSourceFactory;
+                _eventProviderType = providerType;
             }
             internal override void OnControllerCommand(ControllerCommand command, IDictionary<string, string?>? arguments,
                                                               int perEventSourceSessionId)
             {
                 // We use null to represent the ETW EventListener.
                 EventListener? listener = null;
-                m_eventSourceFactory()?.SendCommand(listener, m_eventProviderType, perEventSourceSessionId,
+                _eventSourceFactory()?.SendCommand(listener, _eventProviderType, perEventSourceSessionId,
                                           (EventCommand)command, IsEnabled(), Level, MatchAnyKeyword, arguments);
             }
-            private readonly Func<EventSource?> m_eventSourceFactory;
-            private readonly EventProviderType m_eventProviderType;
+            private readonly Func<EventSource?> _eventSourceFactory;
+            private readonly EventProviderType _eventProviderType;
         }
 
         /// <summary>
@@ -3833,7 +3833,7 @@ internal static void InitializeDefaultEventSources()
             // pulling the System.Diagnostics.DiagnosticSource assembly into the process until it is needed.
             if (AppContext.TryGetSwitch("System.Diagnostics.Metrics.Meter.IsSupported", out bool isSupported) ? isSupported : true)
             {
-                string name = "System.Diagnostics.Metrics";
+                const string name = "System.Diagnostics.Metrics";
                 Guid id = new Guid("20752bc4-c151-50f5-f27b-df92d8af5a61");
                 PreregisterEventProviders(id, name, GetMetricsEventSource);
             }
@@ -3869,6 +3869,10 @@ private static unsafe void PreregisterEventProviders(Guid id, string name, Func<
             //    traits but the SetInformation() call we use below generates metadata from name only. If we want traits support
             //    in the future it could be added.
 
+            // NOTE: You might think this preregister logic could be simplified by using an Action to create the EventSource instead of
+            // Func<EventSource> and then allow the EventSource to initialize as normal. This doesn't work however because calling
+            // EtwEventProvider.Register() inside of an ETW callback deadlocks. Instead we have to bind the EventSource to the
+            // EtwEventProvider that was already registered and use the callback we got on that provider to invoke EventSource.SendCommand().
             try
             {
                 s_preregisteredEventSourceFactories.Add(eventSourceFactory);
@@ -3936,26 +3940,24 @@ internal static void EnsurePreregisteredEventSourcesExist()
         {
             lock (s_preregisteredEtwProviders)
             {
-                s_preregisteredEtwProviders.TryGetValue(id, out OverrideEventProvider? provider);
-                s_preregisteredEtwProviders.Remove(id);
+                s_preregisteredEtwProviders.Remove(id, out OverrideEventProvider? provider);
                 return provider;
             }
         }
 
-        private static Dictionary<Guid, OverrideEventProvider> s_preregisteredEtwProviders = new Dictionary<Guid, OverrideEventProvider>();
+        private static readonly Dictionary<Guid, OverrideEventProvider> s_preregisteredEtwProviders = new Dictionary<Guid, OverrideEventProvider>();
 
 #if FEATURE_PERFTRACING
         private static OverrideEventProvider? TryGetPreregisteredEventPipeProvider(string name)
         {
             lock (s_preregisteredEventPipeProviders)
             {
-                s_preregisteredEventPipeProviders.TryGetValue(name, out OverrideEventProvider? provider);
-                s_preregisteredEventPipeProviders.Remove(name);
+                s_preregisteredEventPipeProviders.Remove(name, out OverrideEventProvider? provider);
                 return provider;
             }
         }
 
-        private static Dictionary<string, OverrideEventProvider> s_preregisteredEventPipeProviders = new Dictionary<string, OverrideEventProvider>();
+        private static readonly Dictionary<string, OverrideEventProvider> s_preregisteredEventPipeProviders = new Dictionary<string, OverrideEventProvider>();
 #endif
 
         // private instance state