From 8fb187b7d12345f94c695c9ad684430ed0f34656 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 10 Jun 2020 12:34:17 -0500 Subject: [PATCH 1/5] Introduce EventSource.IsSupported feature switch This allows for all of the EventSource logic to be trimmed by the ILLinker when the feature switch is set to false. Fix #37414 --- .../ILLink/ILLink.Substitutions.Shared.xml | 4 + .../System/Diagnostics/Tracing/EventSource.cs | 529 ++++++++++-------- .../TraceLogging/TraceLoggingEventSource.cs | 2 +- .../System/Threading/Tasks/TplEventSource.cs | 4 +- 4 files changed, 297 insertions(+), 242 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml index 3af7868a176f86..4afa74a0faa87d 100644 --- a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml +++ b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml @@ -1,5 +1,9 @@ + + + + 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 7216b349a61333..52a8f109537210 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 @@ -233,6 +233,11 @@ public partial class EventSource : IDisposable #pragma warning restore CA1823 #endif //FEATURE_EVENTSOURCE_XPLAT + private static bool IsSupported { get; } = InitializeIsSupported(); + + private static bool InitializeIsSupported() => + AppContext.TryGetSwitch("System.Diagnostics.Tracing.EventSource.IsSupported", out bool isSupported) ? isSupported : true; + /// /// The human-friendly name of the eventSource. It defaults to the simple name of the class /// @@ -277,7 +282,7 @@ public bool IsEnabled(EventLevel level, EventKeywords keywords) /// public bool IsEnabled(EventLevel level, EventKeywords keywords, EventChannel channel) { - if (!m_eventSourceEnabled) + if (!IsEnabled()) return false; if (!IsEnabledCommon(m_eventSourceEnabled, m_level, m_matchAnyKeyword, level, keywords, channel)) @@ -381,6 +386,9 @@ public static string GetName(Type eventSourceType) string? assemblyPathToIncludeInManifest, EventManifestOptions flags) { + if (!IsSupported) + return null; + if (eventSourceType == null) throw new ArgumentNullException(nameof(eventSourceType)); @@ -396,14 +404,17 @@ public static string GetName(Type eventSourceType) public static IEnumerable GetSources() { var ret = new List(); - lock (EventListener.EventListenersLock) + if (IsSupported) { - Debug.Assert(EventListener.s_EventSources != null); - - foreach (WeakReference eventSourceRef in EventListener.s_EventSources) + lock (EventListener.EventListenersLock) { - if (eventSourceRef.TryGetTarget(out EventSource? eventSource) && !eventSource.IsDisposed) - ret.Add(eventSource); + Debug.Assert(EventListener.s_EventSources != null); + + foreach (WeakReference eventSourceRef in EventListener.s_EventSources) + { + if (eventSourceRef.TryGetTarget(out EventSource? eventSource) && !eventSource.IsDisposed) + ret.Add(eventSource); + } } } return ret; @@ -420,16 +431,19 @@ public static IEnumerable GetSources() /// A set of (name-argument, value-argument) pairs associated with the command public static void SendCommand(EventSource eventSource, EventCommand command, IDictionary? commandArguments) { - if (eventSource == null) - throw new ArgumentNullException(nameof(eventSource)); - - // User-defined EventCommands should not conflict with the reserved commands. - if ((int)command <= (int)EventCommand.Update && (int)command != (int)EventCommand.SendManifest) + if (IsSupported) { - throw new ArgumentException(SR.EventSource_InvalidCommand, nameof(command)); - } + if (eventSource == null) + throw new ArgumentNullException(nameof(eventSource)); + + // User-defined EventCommands should not conflict with the reserved commands. + if ((int)command <= (int)EventCommand.Update && (int)command != (int)EventCommand.SendManifest) + { + throw new ArgumentException(SR.EventSource_InvalidCommand, nameof(command)); + } - eventSource.SendCommand(null, EventProviderType.ETW, 0, 0, command, true, EventLevel.LogAlways, EventKeywords.None, commandArguments); + eventSource.SendCommand(null, EventProviderType.ETW, 0, 0, command, true, EventLevel.LogAlways, EventKeywords.None, commandArguments); + } } // Error APIs. (We don't throw by default, but you can probe for status) @@ -501,7 +515,7 @@ public event EventHandler? EventCommandExecuted } } -#region ActivityID + #region ActivityID /// /// When a thread starts work that is on behalf of 'something else' (typically another @@ -522,25 +536,28 @@ public event EventHandler? EventCommandExecuted /// the current thread public static void SetCurrentThreadActivityId(Guid activityId) { - if (TplEventSource.Log != null) - TplEventSource.Log.SetActivityId(activityId); + if (IsSupported) + { + if (TplEventSource.Log != null) + TplEventSource.Log.SetActivityId(activityId); - // We ignore errors to keep with the convention that EventSources do not throw errors. - // Note we can't access m_throwOnWrites because this is a static method. + // We ignore errors to keep with the convention that EventSources do not throw errors. + // Note we can't access m_throwOnWrites because this is a static method. #if FEATURE_MANAGED_ETW #if FEATURE_PERFTRACING - // Set the activity id via EventPipe. - EventPipeInternal.EventActivityIdControl( - (uint)Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, - ref activityId); + // Set the activity id via EventPipe. + EventPipeInternal.EventActivityIdControl( + (uint)Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, + ref activityId); #endif // FEATURE_PERFTRACING #if TARGET_WINDOWS - // Set the activity id via ETW. - Interop.Advapi32.EventActivityIdControl( - Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, - ref activityId); + // Set the activity id via ETW. + Interop.Advapi32.EventActivityIdControl( + Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, + ref activityId); #endif // TARGET_WINDOWS #endif // FEATURE_MANAGED_ETW + } } /// @@ -553,9 +570,11 @@ public static Guid CurrentThreadActivityId // We ignore errors to keep with the convention that EventSources do not throw // errors. Note we can't access m_throwOnWrites because this is a static method. Guid retVal = default; + if (IsSupported) + { #if FEATURE_MANAGED_ETW #if TARGET_WINDOWS - Interop.Advapi32.EventActivityIdControl( + Interop.Advapi32.EventActivityIdControl( Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_GET_ID, ref retVal); #elif FEATURE_PERFTRACING @@ -564,6 +583,7 @@ public static Guid CurrentThreadActivityId ref retVal); #endif // TARGET_WINDOWS #endif // FEATURE_MANAGED_ETW + } return retVal; } } @@ -589,6 +609,12 @@ public static Guid CurrentThreadActivityId /// which will continue at some point in the future, on the current thread public static void SetCurrentThreadActivityId(Guid activityId, out Guid oldActivityThatWillContinue) { + if (!IsSupported) + { + oldActivityThatWillContinue = default; + return; + } + oldActivityThatWillContinue = activityId; #if FEATURE_MANAGED_ETW // We ignore errors to keep with the convention that EventSources do not throw errors. @@ -616,9 +642,9 @@ public static void SetCurrentThreadActivityId(Guid activityId, out Guid oldActiv if (TplEventSource.Log != null) TplEventSource.Log.SetActivityId(activityId); } -#endregion + #endregion -#region protected + #region protected /// /// This is the constructor that most users will use to create their eventSource. It takes /// no parameters. The ETW provider name and GUID of the EventSource are determined by the EventSource @@ -663,13 +689,19 @@ protected EventSource(EventSourceSettings settings) : this(settings, null) { } /// A collection of key-value strings (must be an even number). protected EventSource(EventSourceSettings settings, params string[]? traits) { - m_config = ValidateSettings(settings); + if (IsSupported) + { +#if FEATURE_PERFTRACING + m_eventHandleTable = new TraceLoggingEventHandleTable(); +#endif + m_config = ValidateSettings(settings); - Type myType = this.GetType(); - Guid eventSourceGuid = GetGuid(myType); - string eventSourceName = GetName(myType); + Type myType = this.GetType(); + Guid eventSourceGuid = GetGuid(myType); + string eventSourceName = GetName(myType); - Initialize(eventSourceGuid, eventSourceName, traits); + Initialize(eventSourceGuid, eventSourceName, traits); + } } #if FEATURE_PERFTRACING @@ -734,7 +766,7 @@ protected unsafe void WriteEvent(int eventId) // optimized for common signatures (ints) protected unsafe void WriteEvent(int eventId, int arg1) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[1]; descrs[0].DataPointer = (IntPtr)(&arg1); @@ -746,7 +778,7 @@ protected unsafe void WriteEvent(int eventId, int arg1) protected unsafe void WriteEvent(int eventId, int arg1, int arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[2]; descrs[0].DataPointer = (IntPtr)(&arg1); @@ -761,7 +793,7 @@ protected unsafe void WriteEvent(int eventId, int arg1, int arg2) protected unsafe void WriteEvent(int eventId, int arg1, int arg2, int arg3) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[3]; descrs[0].DataPointer = (IntPtr)(&arg1); @@ -780,7 +812,7 @@ protected unsafe void WriteEvent(int eventId, int arg1, int arg2, int arg3) // optimized for common signatures (longs) protected unsafe void WriteEvent(int eventId, long arg1) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[1]; descrs[0].DataPointer = (IntPtr)(&arg1); @@ -792,7 +824,7 @@ protected unsafe void WriteEvent(int eventId, long arg1) protected unsafe void WriteEvent(int eventId, long arg1, long arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[2]; descrs[0].DataPointer = (IntPtr)(&arg1); @@ -807,7 +839,7 @@ protected unsafe void WriteEvent(int eventId, long arg1, long arg2) protected unsafe void WriteEvent(int eventId, long arg1, long arg2, long arg3) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[3]; descrs[0].DataPointer = (IntPtr)(&arg1); @@ -826,7 +858,7 @@ protected unsafe void WriteEvent(int eventId, long arg1, long arg2, long arg3) // optimized for common signatures (strings) protected unsafe void WriteEvent(int eventId, string? arg1) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg1 ??= ""; fixed (char* string1Bytes = arg1) @@ -842,7 +874,7 @@ protected unsafe void WriteEvent(int eventId, string? arg1) protected unsafe void WriteEvent(int eventId, string? arg1, string? arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg1 ??= ""; arg2 ??= ""; @@ -863,7 +895,7 @@ protected unsafe void WriteEvent(int eventId, string? arg1, string? arg2) protected unsafe void WriteEvent(int eventId, string? arg1, string? arg2, string? arg3) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg1 ??= ""; arg2 ??= ""; @@ -890,7 +922,7 @@ protected unsafe void WriteEvent(int eventId, string? arg1, string? arg2, string // optimized for common signatures (string and ints) protected unsafe void WriteEvent(int eventId, string? arg1, int arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg1 ??= ""; fixed (char* string1Bytes = arg1) @@ -909,7 +941,7 @@ protected unsafe void WriteEvent(int eventId, string? arg1, int arg2) protected unsafe void WriteEvent(int eventId, string? arg1, int arg2, int arg3) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg1 ??= ""; fixed (char* string1Bytes = arg1) @@ -932,7 +964,7 @@ protected unsafe void WriteEvent(int eventId, string? arg1, int arg2, int arg3) // optimized for common signatures (string and longs) protected unsafe void WriteEvent(int eventId, string? arg1, long arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg1 ??= ""; fixed (char* string1Bytes = arg1) @@ -952,7 +984,7 @@ protected unsafe void WriteEvent(int eventId, string? arg1, long arg2) // optimized for common signatures (long and string) protected unsafe void WriteEvent(int eventId, long arg1, string? arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg2 ??= ""; fixed (char* string2Bytes = arg2) @@ -972,7 +1004,7 @@ protected unsafe void WriteEvent(int eventId, long arg1, string? arg2) // optimized for common signatures (int and string) protected unsafe void WriteEvent(int eventId, int arg1, string? arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { arg2 ??= ""; fixed (char* string2Bytes = arg2) @@ -991,7 +1023,7 @@ protected unsafe void WriteEvent(int eventId, int arg1, string? arg2) protected unsafe void WriteEvent(int eventId, byte[]? arg1) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[2]; if (arg1 == null || arg1.Length == 0) @@ -1024,7 +1056,7 @@ protected unsafe void WriteEvent(int eventId, byte[]? arg1) protected unsafe void WriteEvent(int eventId, long arg1, byte[]? arg2) { - if (m_eventSourceEnabled) + if (IsEnabled()) { EventSource.EventData* descrs = stackalloc EventSource.EventData[3]; descrs[0].DataPointer = (IntPtr)(&arg1); @@ -1094,7 +1126,7 @@ internal int Reserved set => m_Reserved = value; } -#region private + #region private /// /// Initializes the members of this EventData object to point at a previously-pinned /// tracelogging-compatible metadata blob. @@ -1116,7 +1148,7 @@ internal unsafe void SetMetadata(byte* pointer, int size, int reserved) #pragma warning disable 0649 internal int m_Reserved; // Used to pad the size to match the Win32 API #pragma warning restore 0649 -#endregion + #endregion } /// @@ -1179,7 +1211,7 @@ protected unsafe void WriteEventCore(int eventId, int eventDataCount, EventSourc [CLSCompliant(false)] protected unsafe void WriteEventWithRelatedActivityIdCore(int eventId, Guid* relatedActivityId, int eventDataCount, EventSource.EventData* data) { - if (m_eventSourceEnabled) + if (IsEnabled()) { Debug.Assert(m_eventData != null); // You must have initialized this if you enabled the source. try @@ -1288,9 +1320,9 @@ protected unsafe void WriteEventWithRelatedActivityId(int eventId, Guid relatedA WriteEventVarargs(eventId, &relatedActivityId, args); } -#endregion + #endregion -#region IDisposable Members + #region IDisposable Members /// /// Disposes of an EventSource. /// @@ -1311,36 +1343,39 @@ public void Dispose() /// True if called from Dispose(), false if called from the finalizer. protected virtual void Dispose(bool disposing) { - if (disposing) + if (EventSource.IsSupported) { -#if FEATURE_MANAGED_ETW - // Send the manifest one more time to ensure circular buffers have a chance to get to this information - // even in scenarios with a high volume of ETW events. - if (m_eventSourceEnabled) + if (disposing) { - try +#if FEATURE_MANAGED_ETW + // Send the manifest one more time to ensure circular buffers have a chance to get to this information + // even in scenarios with a high volume of ETW events. + if (m_eventSourceEnabled) + { + try + { + SendManifest(m_rawManifest); + } + catch { } // If it fails, simply give up. + m_eventSourceEnabled = false; + } + if (m_etwProvider != null) { - SendManifest(m_rawManifest); + m_etwProvider.Dispose(); + m_etwProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose } - catch { } // If it fails, simply give up. - m_eventSourceEnabled = false; - } - if (m_etwProvider != null) - { - m_etwProvider.Dispose(); - m_etwProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose - } #endif #if FEATURE_PERFTRACING - if (m_eventPipeProvider != null) - { - m_eventPipeProvider.Dispose(); - m_eventPipeProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose - } + if (m_eventPipeProvider != null) + { + m_eventPipeProvider.Dispose(); + m_eventPipeProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose + } #endif + } + m_eventSourceEnabled = false; + m_eventSourceDisposed = true; } - m_eventSourceEnabled = false; - m_eventSourceDisposed = true; } /// /// Finalizer for EventSource @@ -1349,9 +1384,9 @@ protected virtual void Dispose(bool disposing) { this.Dispose(false); } -#endregion + #endregion -#region private + #region private private unsafe void WriteEventRaw( string? eventName, @@ -1396,8 +1431,15 @@ internal EventSource(Guid eventSourceGuid, string eventSourceName) // Used by the internal FrameworkEventSource constructor and the TraceLogging-style event source constructor internal EventSource(Guid eventSourceGuid, string eventSourceName, EventSourceSettings settings, string[]? traits = null) { - m_config = ValidateSettings(settings); - Initialize(eventSourceGuid, eventSourceName, traits); + if (IsSupported) + { +#if FEATURE_PERFTRACING + m_eventHandleTable = new TraceLoggingEventHandleTable(); +#endif + + m_config = ValidateSettings(settings); + Initialize(eventSourceGuid, eventSourceName, traits); + } } /// @@ -1725,7 +1767,7 @@ private static Guid GenerateGuidFromName(string name) Debug.Assert(m_eventData != null); Type dataType = GetDataType(m_eventData[eventId], parameterId); - Again: + Again: if (dataType == typeof(IntPtr)) { return *((IntPtr*)dataPointer); @@ -1862,7 +1904,7 @@ private static Guid GenerateGuidFromName(string name) private unsafe void WriteEventVarargs(int eventId, Guid* childActivityID, object?[] args) { - if (m_eventSourceEnabled) + if (IsEnabled()) { Debug.Assert(m_eventData != null); // You must have initialized this if you enabled the source. try @@ -2506,30 +2548,33 @@ internal void SendCommand(EventListener? listener, EventProviderType eventProvid EventLevel level, EventKeywords matchAnyKeyword, IDictionary? commandArguments) { - var commandArgs = new EventCommandEventArgs(command, commandArguments, this, listener, eventProviderType, perEventSourceSessionId, etwSessionId, enable, level, matchAnyKeyword); - lock (EventListener.EventListenersLock) + if (IsSupported) { - if (m_completelyInited) - { - // After the first command arrive after construction, we are ready to get rid of the deferred commands - this.m_deferredCommands = null; - // We are fully initialized, do the command - DoCommand(commandArgs); - } - else + var commandArgs = new EventCommandEventArgs(command, commandArguments, this, listener, eventProviderType, perEventSourceSessionId, etwSessionId, enable, level, matchAnyKeyword); + lock (EventListener.EventListenersLock) { - // We can't do the command, simply remember it and we do it when we are fully constructed. - if (m_deferredCommands == null) + if (m_completelyInited) { - m_deferredCommands = commandArgs; // create the first entry + // After the first command arrive after construction, we are ready to get rid of the deferred commands + this.m_deferredCommands = null; + // We are fully initialized, do the command + DoCommand(commandArgs); } else { - // We have one or more entries, find the last one and add it to that. - EventCommandEventArgs lastCommand = m_deferredCommands; - while (lastCommand.nextCommand != null) - lastCommand = lastCommand.nextCommand; - lastCommand.nextCommand = commandArgs; + // We can't do the command, simply remember it and we do it when we are fully constructed. + if (m_deferredCommands == null) + { + m_deferredCommands = commandArgs; // create the first entry + } + else + { + // We have one or more entries, find the last one and add it to that. + EventCommandEventArgs lastCommand = m_deferredCommands; + while (lastCommand.nextCommand != null) + lastCommand = lastCommand.nextCommand; + lastCommand.nextCommand = commandArgs; + } } } } @@ -2543,162 +2588,165 @@ internal void SendCommand(EventListener? listener, EventProviderType eventProvid /// 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); + if (IsSupported) + { + // 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); #if FEATURE_MANAGED_ETW - if (m_etwProvider == null) // If we failed to construct - return; + if (m_etwProvider == null) // If we failed to construct + return; #endif // FEATURE_MANAGED_ETW #if FEATURE_PERFTRACING - if (m_eventPipeProvider == null) - return; + if (m_eventPipeProvider == null) + return; #endif - m_outOfBandMessageCount = 0; - try - { - EnsureDescriptorsInitialized(); - Debug.Assert(m_eventData != null); - - // Find the per-EventSource dispatcher corresponding to registered dispatcher - commandArgs.dispatcher = GetDispatcher(commandArgs.listener); - if (commandArgs.dispatcher == null && commandArgs.listener != null) // dispatcher == null means ETW dispatcher + m_outOfBandMessageCount = 0; + try { - throw new ArgumentException(SR.EventSource_ListenerNotFound); - } + EnsureDescriptorsInitialized(); + Debug.Assert(m_eventData != null); - commandArgs.Arguments ??= new Dictionary(); + // Find the per-EventSource dispatcher corresponding to registered dispatcher + commandArgs.dispatcher = GetDispatcher(commandArgs.listener); + if (commandArgs.dispatcher == null && commandArgs.listener != null) // dispatcher == null means ETW dispatcher + { + throw new ArgumentException(SR.EventSource_ListenerNotFound); + } - if (commandArgs.Command == EventCommand.Update) - { - // Set it up using the 'standard' filtering bitfields (use the "global" enable, not session specific one) - for (int i = 0; i < m_eventData.Length; i++) - EnableEventForDispatcher(commandArgs.dispatcher, commandArgs.eventProviderType, i, IsEnabledByDefault(i, commandArgs.enable, commandArgs.level, commandArgs.matchAnyKeyword)); + commandArgs.Arguments ??= new Dictionary(); - if (commandArgs.enable) + if (commandArgs.Command == EventCommand.Update) { - if (!m_eventSourceEnabled) - { - // EventSource turned on for the first time, simply copy the bits. - m_level = commandArgs.level; - m_matchAnyKeyword = commandArgs.matchAnyKeyword; - } - else + // Set it up using the 'standard' filtering bitfields (use the "global" enable, not session specific one) + for (int i = 0; i < m_eventData.Length; i++) + EnableEventForDispatcher(commandArgs.dispatcher, commandArgs.eventProviderType, i, IsEnabledByDefault(i, commandArgs.enable, commandArgs.level, commandArgs.matchAnyKeyword)); + + if (commandArgs.enable) { - // Already enabled, make it the most verbose of the existing and new filter - if (commandArgs.level > m_level) + if (!m_eventSourceEnabled) + { + // EventSource turned on for the first time, simply copy the bits. m_level = commandArgs.level; - if (commandArgs.matchAnyKeyword == 0) - m_matchAnyKeyword = 0; - else if (m_matchAnyKeyword != 0) - m_matchAnyKeyword = unchecked(m_matchAnyKeyword | commandArgs.matchAnyKeyword); + m_matchAnyKeyword = commandArgs.matchAnyKeyword; + } + else + { + // Already enabled, make it the most verbose of the existing and new filter + if (commandArgs.level > m_level) + m_level = commandArgs.level; + if (commandArgs.matchAnyKeyword == 0) + m_matchAnyKeyword = 0; + else if (m_matchAnyKeyword != 0) + m_matchAnyKeyword = unchecked(m_matchAnyKeyword | commandArgs.matchAnyKeyword); + } } - } - // interpret perEventSourceSessionId's sign, and adjust perEventSourceSessionId to - // represent 0-based positive values - bool bSessionEnable = (commandArgs.perEventSourceSessionId >= 0); - if (commandArgs.perEventSourceSessionId == 0 && !commandArgs.enable) - bSessionEnable = false; + // interpret perEventSourceSessionId's sign, and adjust perEventSourceSessionId to + // represent 0-based positive values + bool bSessionEnable = (commandArgs.perEventSourceSessionId >= 0); + if (commandArgs.perEventSourceSessionId == 0 && !commandArgs.enable) + bSessionEnable = false; - if (commandArgs.listener == null) - { - if (!bSessionEnable) - commandArgs.perEventSourceSessionId = -commandArgs.perEventSourceSessionId; - // for "global" enable/disable (passed in with listener == null and - // perEventSourceSessionId == 0) perEventSourceSessionId becomes -1 - --commandArgs.perEventSourceSessionId; - } - - commandArgs.Command = bSessionEnable ? EventCommand.Enable : EventCommand.Disable; + if (commandArgs.listener == null) + { + if (!bSessionEnable) + commandArgs.perEventSourceSessionId = -commandArgs.perEventSourceSessionId; + // for "global" enable/disable (passed in with listener == null and + // perEventSourceSessionId == 0) perEventSourceSessionId becomes -1 + --commandArgs.perEventSourceSessionId; + } - // perEventSourceSessionId = -1 when ETW sent a notification, but the set of active sessions - // hasn't changed. - // sesisonId = SessionMask.MAX when one of the legacy ETW sessions changed - // 0 <= perEventSourceSessionId < SessionMask.MAX for activity-tracing aware sessions - Debug.Assert(commandArgs.perEventSourceSessionId >= -1 && commandArgs.perEventSourceSessionId <= SessionMask.MAX); + commandArgs.Command = bSessionEnable ? EventCommand.Enable : EventCommand.Disable; - // Send the manifest if we are enabling an ETW session - if (bSessionEnable && commandArgs.dispatcher == null) - { - // eventSourceDispatcher == null means this is the ETW manifest + // perEventSourceSessionId = -1 when ETW sent a notification, but the set of active sessions + // hasn't changed. + // sesisonId = SessionMask.MAX when one of the legacy ETW sessions changed + // 0 <= perEventSourceSessionId < SessionMask.MAX for activity-tracing aware sessions + Debug.Assert(commandArgs.perEventSourceSessionId >= -1 && commandArgs.perEventSourceSessionId <= SessionMask.MAX); - // Note that we unconditionally send the manifest whenever we are enabled, even if - // we were already enabled. This is because there may be multiple sessions active - // and we can't know that all the sessions have seen the manifest. - if (!SelfDescribingEvents) - SendManifest(m_rawManifest); - } + // Send the manifest if we are enabling an ETW session + if (bSessionEnable && commandArgs.dispatcher == null) + { + // eventSourceDispatcher == null means this is the ETW manifest - // Turn on the enable bit before making the OnEventCommand callback This allows you to do useful - // things like log messages, or test if keywords are enabled in the callback. - if (commandArgs.enable) - { - Debug.Assert(m_eventData != null); - m_eventSourceEnabled = true; - } + // Note that we unconditionally send the manifest whenever we are enabled, even if + // we were already enabled. This is because there may be multiple sessions active + // and we can't know that all the sessions have seen the manifest. + if (!SelfDescribingEvents) + SendManifest(m_rawManifest); + } - this.OnEventCommand(commandArgs); - this.m_eventCommandExecuted?.Invoke(this, commandArgs); + // Turn on the enable bit before making the OnEventCommand callback This allows you to do useful + // things like log messages, or test if keywords are enabled in the callback. + if (commandArgs.enable) + { + Debug.Assert(m_eventData != null); + m_eventSourceEnabled = true; + } - if (!commandArgs.enable) - { - // If we are disabling, maybe we can turn on 'quick checks' to filter - // quickly. These are all just optimizations (since later checks will still filter) + this.OnEventCommand(commandArgs); + this.m_eventCommandExecuted?.Invoke(this, commandArgs); - // There is a good chance EnabledForAnyListener are not as accurate as - // they could be, go ahead and get a better estimate. - for (int i = 0; i < m_eventData.Length; i++) + if (!commandArgs.enable) { - bool isEnabledForAnyListener = false; - for (EventDispatcher? dispatcher = m_Dispatchers; dispatcher != null; dispatcher = dispatcher.m_Next) - { - Debug.Assert(dispatcher.m_EventEnabled != null); + // If we are disabling, maybe we can turn on 'quick checks' to filter + // quickly. These are all just optimizations (since later checks will still filter) - if (dispatcher.m_EventEnabled[i]) + // There is a good chance EnabledForAnyListener are not as accurate as + // they could be, go ahead and get a better estimate. + for (int i = 0; i < m_eventData.Length; i++) + { + bool isEnabledForAnyListener = false; + for (EventDispatcher? dispatcher = m_Dispatchers; dispatcher != null; dispatcher = dispatcher.m_Next) { - isEnabledForAnyListener = true; - break; + Debug.Assert(dispatcher.m_EventEnabled != null); + + if (dispatcher.m_EventEnabled[i]) + { + isEnabledForAnyListener = true; + break; + } } + m_eventData[i].EnabledForAnyListener = isEnabledForAnyListener; } - m_eventData[i].EnabledForAnyListener = isEnabledForAnyListener; - } - // If no events are enabled, disable the global enabled bit. - if (!AnyEventEnabled()) - { - m_level = 0; - m_matchAnyKeyword = 0; - m_eventSourceEnabled = false; + // If no events are enabled, disable the global enabled bit. + if (!AnyEventEnabled()) + { + m_level = 0; + m_matchAnyKeyword = 0; + m_eventSourceEnabled = false; + } } } - } - else - { - if (commandArgs.Command == EventCommand.SendManifest) + else { - // TODO: should we generate the manifest here if we hadn't already? - if (m_rawManifest != null) - SendManifest(m_rawManifest); - } + if (commandArgs.Command == EventCommand.SendManifest) + { + // TODO: should we generate the manifest here if we hadn't already? + if (m_rawManifest != null) + SendManifest(m_rawManifest); + } - // These are not used for non-update commands and thus should always be 'default' values - // Debug.Assert(enable == true); - // Debug.Assert(level == EventLevel.LogAlways); - // Debug.Assert(matchAnyKeyword == EventKeywords.None); + // These are not used for non-update commands and thus should always be 'default' values + // Debug.Assert(enable == true); + // Debug.Assert(level == EventLevel.LogAlways); + // Debug.Assert(matchAnyKeyword == EventKeywords.None); - this.OnEventCommand(commandArgs); - m_eventCommandExecuted?.Invoke(this, commandArgs); + this.OnEventCommand(commandArgs); + m_eventCommandExecuted?.Invoke(this, commandArgs); + } + } + catch (Exception e) + { + // When the ETW session is created after the EventSource has registered with the ETW system + // we can send any error messages here. + ReportOutOfBandMessage("ERROR: Exception in Command Processing for EventSource " + Name + ": " + e.Message); + // We never throw when doing a command. } - } - catch (Exception e) - { - // When the ETW session is created after the EventSource has registered with the ETW system - // we can send any error messages here. - ReportOutOfBandMessage("ERROR: Exception in Command Processing for EventSource " + Name + ": " + e.Message); - // We never throw when doing a command. } } @@ -2709,6 +2757,9 @@ internal void DoCommand(EventCommandEventArgs commandArgs) /// internal bool EnableEventForDispatcher(EventDispatcher? dispatcher, EventProviderType eventProviderType, int eventId, bool value) { + if (!IsSupported) + return false; + Debug.Assert(m_eventData != null); if (dispatcher == null) @@ -2836,7 +2887,7 @@ private unsafe void SendManifest(byte[]? rawManifest) dataDescrs[1].Reserved = 0; int chunkSize = ManifestEnvelope.MaxChunkSize; - TRY_AGAIN_WITH_SMALLER_CHUNK_SIZE: + TRY_AGAIN_WITH_SMALLER_CHUNK_SIZE: envelope.TotalChunks = (ushort)((dataLeft + (chunkSize - 1)) / chunkSize); while (dataLeft > 0) { @@ -3375,7 +3426,7 @@ private static void AddProviderEnumKind(ManifestBuilder manifest, FieldInfo stat } #endif return; - Error: + Error: manifest.ManifestError(SR.Format(SR.EventSource_EnumKindMismatch, staticField.Name, staticField.FieldType.Name, providerEnumKind)); } @@ -3780,7 +3831,7 @@ private bool SelfDescribingEvents // used for generating GUID from eventsource name private static byte[]? namespaceBytes; #endif -#endregion + #endregion } /// @@ -4074,7 +4125,7 @@ protected internal virtual void OnEventWritten(EventWrittenEventArgs eventData) this.EventWritten?.Invoke(this, eventData); } -#region private + #region private /// /// This routine adds newEventSource to the global list of eventSources, it also assigns the /// ID to the eventSource (which is simply the ordinal in the global list). @@ -4399,7 +4450,7 @@ private void CallBackForExistingEventSources(bool addToListenersList, EventHandl /// Used to register AD/Process shutdown callbacks. /// private static bool s_EventSourceShutdownRegistered = false; -#endregion + #endregion } /// @@ -4441,7 +4492,7 @@ public bool DisableEvent(int eventId) return eventSource.EnableEventForDispatcher(dispatcher, eventProviderType, eventId, false); } -#region private + #region private internal EventCommandEventArgs(EventCommand command, IDictionary? arguments, EventSource eventSource, EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId, int etwSessionId, bool enable, EventLevel level, EventKeywords matchAnyKeyword) @@ -4471,7 +4522,7 @@ internal EventCommandEventArgs(EventCommand command, IDictionary @@ -4742,7 +4793,7 @@ public DateTime TimeStamp internal set; } -#region private + #region private internal EventWrittenEventArgs(EventSource eventSource) { m_eventSource = eventSource; @@ -4758,7 +4809,7 @@ internal EventWrittenEventArgs(EventSource eventSource) internal EventOpcode m_opcode; internal EventLevel m_level; internal EventKeywords m_keywords; -#endregion + #endregion } /// @@ -4861,10 +4912,10 @@ public EventOpcode Opcode /// public EventActivityOptions ActivityOptions { get; set; } -#region private + #region private private EventOpcode m_opcode; private bool m_opcodeSet; -#endregion + #endregion } /// @@ -5007,7 +5058,7 @@ public enum EventCommand Disable = -3 } -#region private classes + #region private classes // holds a bitfield representing a session mask /// @@ -5682,7 +5733,7 @@ private string CreateManifestString() return sb.ToString(); } -#region private + #region private private void WriteNameAndMessageAttribs(StringBuilder stringBuilder, string elementName, string name) { stringBuilder.Append(" name=\"").Append(name).Append('"'); @@ -6061,7 +6112,7 @@ private class ChannelInfo private string? eventName; // Name of the event currently being processed. private int numParams; // keeps track of the number of args the event has. private List? byteArrArgIndices; // keeps track of the index of each byte[] argument -#endregion + #endregion } /// @@ -6085,5 +6136,5 @@ public enum ManifestFormats : byte #endif } -#endregion + #endregion } diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs index 168e529daa84e8..3eaa3753991361 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs @@ -38,7 +38,7 @@ public partial class EventSource #endif #if FEATURE_PERFTRACING - private readonly TraceLoggingEventHandleTable m_eventHandleTable = new TraceLoggingEventHandleTable(); + private readonly TraceLoggingEventHandleTable m_eventHandleTable = null!; #endif /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs index c1a1d20b0e1d84..e81d5c67f51a76 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs @@ -269,7 +269,7 @@ public void TaskCompleted( int OriginatingTaskSchedulerID, int OriginatingTaskID, // PFX_COMMON_EVENT_HEADER int TaskID, bool IsExceptional) { - if (IsEnabled(EventLevel.Informational, Keywords.Tasks)) + if (IsEnabled() && IsEnabled(EventLevel.Informational, Keywords.Tasks)) { unsafe { @@ -538,7 +538,7 @@ public void NewID(int TaskID) public void IncompleteAsyncMethod(IAsyncStateMachineBox stateMachineBox) { System.Diagnostics.Debug.Assert(stateMachineBox != null); - if (IsEnabled(EventLevel.Warning, Keywords.AsyncMethod)) + if (IsEnabled() && IsEnabled(EventLevel.Warning, Keywords.AsyncMethod)) { IAsyncStateMachine stateMachine = stateMachineBox.GetStateMachineObject(); if (stateMachine != null) From bb7f5f344b48a2ee6b4511b3b40e245a59d582d2 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 22 Jun 2020 18:59:05 -0500 Subject: [PATCH 2/5] First round of PR feedback. --- .../ILLink/ILLink.Substitutions.Shared.xml | 2 + .../System/Diagnostics/Tracing/EventSource.cs | 470 +++++++++--------- 2 files changed, 245 insertions(+), 227 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml index 4afa74a0faa87d..44149875d2b0a4 100644 --- a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml +++ b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml @@ -3,6 +3,8 @@ + + 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 52a8f109537210..b9e9dd378993ca 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 @@ -387,7 +387,9 @@ public static string GetName(Type eventSourceType) EventManifestOptions flags) { if (!IsSupported) + { return null; + } if (eventSourceType == null) throw new ArgumentNullException(nameof(eventSourceType)); @@ -403,18 +405,20 @@ public static string GetName(Type eventSourceType) /// public static IEnumerable GetSources() { - var ret = new List(); if (IsSupported) { - lock (EventListener.EventListenersLock) - { - Debug.Assert(EventListener.s_EventSources != null); + return Array.Empty(); + } - foreach (WeakReference eventSourceRef in EventListener.s_EventSources) - { - if (eventSourceRef.TryGetTarget(out EventSource? eventSource) && !eventSource.IsDisposed) - ret.Add(eventSource); - } + var ret = new List(); + lock (EventListener.EventListenersLock) + { + Debug.Assert(EventListener.s_EventSources != null); + + foreach (WeakReference eventSourceRef in EventListener.s_EventSources) + { + if (eventSourceRef.TryGetTarget(out EventSource? eventSource) && !eventSource.IsDisposed) + ret.Add(eventSource); } } return ret; @@ -431,19 +435,21 @@ public static IEnumerable GetSources() /// A set of (name-argument, value-argument) pairs associated with the command public static void SendCommand(EventSource eventSource, EventCommand command, IDictionary? commandArguments) { - if (IsSupported) + if (!IsSupported) { - if (eventSource == null) - throw new ArgumentNullException(nameof(eventSource)); + return; + } - // User-defined EventCommands should not conflict with the reserved commands. - if ((int)command <= (int)EventCommand.Update && (int)command != (int)EventCommand.SendManifest) - { - throw new ArgumentException(SR.EventSource_InvalidCommand, nameof(command)); - } + if (eventSource == null) + throw new ArgumentNullException(nameof(eventSource)); - eventSource.SendCommand(null, EventProviderType.ETW, 0, 0, command, true, EventLevel.LogAlways, EventKeywords.None, commandArguments); + // User-defined EventCommands should not conflict with the reserved commands. + if ((int)command <= (int)EventCommand.Update && (int)command != (int)EventCommand.SendManifest) + { + throw new ArgumentException(SR.EventSource_InvalidCommand, nameof(command)); } + + eventSource.SendCommand(null, EventProviderType.ETW, 0, 0, command, true, EventLevel.LogAlways, EventKeywords.None, commandArguments); } // Error APIs. (We don't throw by default, but you can probe for status) @@ -515,7 +521,7 @@ public event EventHandler? EventCommandExecuted } } - #region ActivityID +#region ActivityID /// /// When a thread starts work that is on behalf of 'something else' (typically another @@ -536,28 +542,30 @@ public event EventHandler? EventCommandExecuted /// the current thread public static void SetCurrentThreadActivityId(Guid activityId) { - if (IsSupported) + if (!IsSupported) { - if (TplEventSource.Log != null) - TplEventSource.Log.SetActivityId(activityId); + return; + } - // We ignore errors to keep with the convention that EventSources do not throw errors. - // Note we can't access m_throwOnWrites because this is a static method. + if (TplEventSource.Log != null) + TplEventSource.Log.SetActivityId(activityId); + + // We ignore errors to keep with the convention that EventSources do not throw errors. + // Note we can't access m_throwOnWrites because this is a static method. #if FEATURE_MANAGED_ETW #if FEATURE_PERFTRACING - // Set the activity id via EventPipe. - EventPipeInternal.EventActivityIdControl( - (uint)Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, - ref activityId); + // Set the activity id via EventPipe. + EventPipeInternal.EventActivityIdControl( + (uint)Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, + ref activityId); #endif // FEATURE_PERFTRACING #if TARGET_WINDOWS - // Set the activity id via ETW. - Interop.Advapi32.EventActivityIdControl( - Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, - ref activityId); + // Set the activity id via ETW. + Interop.Advapi32.EventActivityIdControl( + Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_SET_ID, + ref activityId); #endif // TARGET_WINDOWS #endif // FEATURE_MANAGED_ETW - } } /// @@ -567,14 +575,17 @@ public static Guid CurrentThreadActivityId { get { + if (!IsSupported) + { + return default; + } + // We ignore errors to keep with the convention that EventSources do not throw // errors. Note we can't access m_throwOnWrites because this is a static method. Guid retVal = default; - if (IsSupported) - { #if FEATURE_MANAGED_ETW #if TARGET_WINDOWS - Interop.Advapi32.EventActivityIdControl( + Interop.Advapi32.EventActivityIdControl( Interop.Advapi32.ActivityControl.EVENT_ACTIVITY_CTRL_GET_ID, ref retVal); #elif FEATURE_PERFTRACING @@ -583,7 +594,6 @@ public static Guid CurrentThreadActivityId ref retVal); #endif // TARGET_WINDOWS #endif // FEATURE_MANAGED_ETW - } return retVal; } } @@ -642,9 +652,9 @@ public static void SetCurrentThreadActivityId(Guid activityId, out Guid oldActiv if (TplEventSource.Log != null) TplEventSource.Log.SetActivityId(activityId); } - #endregion +#endregion - #region protected +#region protected /// /// This is the constructor that most users will use to create their eventSource. It takes /// no parameters. The ETW provider name and GUID of the EventSource are determined by the EventSource @@ -1126,7 +1136,7 @@ internal int Reserved set => m_Reserved = value; } - #region private +#region private /// /// Initializes the members of this EventData object to point at a previously-pinned /// tracelogging-compatible metadata blob. @@ -1148,7 +1158,7 @@ internal unsafe void SetMetadata(byte* pointer, int size, int reserved) #pragma warning disable 0649 internal int m_Reserved; // Used to pad the size to match the Win32 API #pragma warning restore 0649 - #endregion +#endregion } /// @@ -1320,9 +1330,9 @@ protected unsafe void WriteEventWithRelatedActivityId(int eventId, Guid relatedA WriteEventVarargs(eventId, &relatedActivityId, args); } - #endregion +#endregion - #region IDisposable Members +#region IDisposable Members /// /// Disposes of an EventSource. /// @@ -1343,39 +1353,41 @@ public void Dispose() /// True if called from Dispose(), false if called from the finalizer. protected virtual void Dispose(bool disposing) { - if (EventSource.IsSupported) + if (!IsSupported) + { + return; + } + + if (disposing) { - if (disposing) - { #if FEATURE_MANAGED_ETW - // Send the manifest one more time to ensure circular buffers have a chance to get to this information - // even in scenarios with a high volume of ETW events. - if (m_eventSourceEnabled) - { - try - { - SendManifest(m_rawManifest); - } - catch { } // If it fails, simply give up. - m_eventSourceEnabled = false; - } - if (m_etwProvider != null) + // Send the manifest one more time to ensure circular buffers have a chance to get to this information + // even in scenarios with a high volume of ETW events. + if (m_eventSourceEnabled) + { + try { - m_etwProvider.Dispose(); - m_etwProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose + SendManifest(m_rawManifest); } + catch { } // If it fails, simply give up. + m_eventSourceEnabled = false; + } + if (m_etwProvider != null) + { + m_etwProvider.Dispose(); + m_etwProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose + } #endif #if FEATURE_PERFTRACING - if (m_eventPipeProvider != null) - { - m_eventPipeProvider.Dispose(); - m_eventPipeProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose - } -#endif + if (m_eventPipeProvider != null) + { + m_eventPipeProvider.Dispose(); + m_eventPipeProvider = null!; // TODO-NULLABLE: Avoid nulling out in Dispose } - m_eventSourceEnabled = false; - m_eventSourceDisposed = true; +#endif } + m_eventSourceEnabled = false; + m_eventSourceDisposed = true; } /// /// Finalizer for EventSource @@ -1384,9 +1396,9 @@ protected virtual void Dispose(bool disposing) { this.Dispose(false); } - #endregion +#endregion - #region private +#region private private unsafe void WriteEventRaw( string? eventName, @@ -2548,33 +2560,35 @@ internal void SendCommand(EventListener? listener, EventProviderType eventProvid EventLevel level, EventKeywords matchAnyKeyword, IDictionary? commandArguments) { - if (IsSupported) + if (!IsSupported) { - var commandArgs = new EventCommandEventArgs(command, commandArguments, this, listener, eventProviderType, perEventSourceSessionId, etwSessionId, enable, level, matchAnyKeyword); - lock (EventListener.EventListenersLock) + return; + } + + var commandArgs = new EventCommandEventArgs(command, commandArguments, this, listener, eventProviderType, perEventSourceSessionId, etwSessionId, enable, level, matchAnyKeyword); + lock (EventListener.EventListenersLock) + { + if (m_completelyInited) + { + // After the first command arrive after construction, we are ready to get rid of the deferred commands + this.m_deferredCommands = null; + // We are fully initialized, do the command + DoCommand(commandArgs); + } + else { - if (m_completelyInited) + // We can't do the command, simply remember it and we do it when we are fully constructed. + if (m_deferredCommands == null) { - // After the first command arrive after construction, we are ready to get rid of the deferred commands - this.m_deferredCommands = null; - // We are fully initialized, do the command - DoCommand(commandArgs); + m_deferredCommands = commandArgs; // create the first entry } else { - // We can't do the command, simply remember it and we do it when we are fully constructed. - if (m_deferredCommands == null) - { - m_deferredCommands = commandArgs; // create the first entry - } - else - { - // We have one or more entries, find the last one and add it to that. - EventCommandEventArgs lastCommand = m_deferredCommands; - while (lastCommand.nextCommand != null) - lastCommand = lastCommand.nextCommand; - lastCommand.nextCommand = commandArgs; - } + // We have one or more entries, find the last one and add it to that. + EventCommandEventArgs lastCommand = m_deferredCommands; + while (lastCommand.nextCommand != null) + lastCommand = lastCommand.nextCommand; + lastCommand.nextCommand = commandArgs; } } } @@ -2588,166 +2602,168 @@ internal void SendCommand(EventListener? listener, EventProviderType eventProvid /// internal void DoCommand(EventCommandEventArgs commandArgs) { - if (IsSupported) + if (!IsSupported) { - // 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); + return; + } + + // 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); #if FEATURE_MANAGED_ETW - if (m_etwProvider == null) // If we failed to construct - return; + if (m_etwProvider == null) // If we failed to construct + return; #endif // FEATURE_MANAGED_ETW #if FEATURE_PERFTRACING - if (m_eventPipeProvider == null) - return; + if (m_eventPipeProvider == null) + return; #endif - m_outOfBandMessageCount = 0; - try + m_outOfBandMessageCount = 0; + try + { + EnsureDescriptorsInitialized(); + Debug.Assert(m_eventData != null); + + // Find the per-EventSource dispatcher corresponding to registered dispatcher + commandArgs.dispatcher = GetDispatcher(commandArgs.listener); + if (commandArgs.dispatcher == null && commandArgs.listener != null) // dispatcher == null means ETW dispatcher { - EnsureDescriptorsInitialized(); - Debug.Assert(m_eventData != null); + throw new ArgumentException(SR.EventSource_ListenerNotFound); + } - // Find the per-EventSource dispatcher corresponding to registered dispatcher - commandArgs.dispatcher = GetDispatcher(commandArgs.listener); - if (commandArgs.dispatcher == null && commandArgs.listener != null) // dispatcher == null means ETW dispatcher - { - throw new ArgumentException(SR.EventSource_ListenerNotFound); - } + commandArgs.Arguments ??= new Dictionary(); - commandArgs.Arguments ??= new Dictionary(); + if (commandArgs.Command == EventCommand.Update) + { + // Set it up using the 'standard' filtering bitfields (use the "global" enable, not session specific one) + for (int i = 0; i < m_eventData.Length; i++) + EnableEventForDispatcher(commandArgs.dispatcher, commandArgs.eventProviderType, i, IsEnabledByDefault(i, commandArgs.enable, commandArgs.level, commandArgs.matchAnyKeyword)); - if (commandArgs.Command == EventCommand.Update) + if (commandArgs.enable) { - // Set it up using the 'standard' filtering bitfields (use the "global" enable, not session specific one) - for (int i = 0; i < m_eventData.Length; i++) - EnableEventForDispatcher(commandArgs.dispatcher, commandArgs.eventProviderType, i, IsEnabledByDefault(i, commandArgs.enable, commandArgs.level, commandArgs.matchAnyKeyword)); - - if (commandArgs.enable) + if (!m_eventSourceEnabled) { - if (!m_eventSourceEnabled) - { - // EventSource turned on for the first time, simply copy the bits. + // EventSource turned on for the first time, simply copy the bits. + m_level = commandArgs.level; + m_matchAnyKeyword = commandArgs.matchAnyKeyword; + } + else + { + // Already enabled, make it the most verbose of the existing and new filter + if (commandArgs.level > m_level) m_level = commandArgs.level; - m_matchAnyKeyword = commandArgs.matchAnyKeyword; - } - else - { - // Already enabled, make it the most verbose of the existing and new filter - if (commandArgs.level > m_level) - m_level = commandArgs.level; - if (commandArgs.matchAnyKeyword == 0) - m_matchAnyKeyword = 0; - else if (m_matchAnyKeyword != 0) - m_matchAnyKeyword = unchecked(m_matchAnyKeyword | commandArgs.matchAnyKeyword); - } + if (commandArgs.matchAnyKeyword == 0) + m_matchAnyKeyword = 0; + else if (m_matchAnyKeyword != 0) + m_matchAnyKeyword = unchecked(m_matchAnyKeyword | commandArgs.matchAnyKeyword); } + } - // interpret perEventSourceSessionId's sign, and adjust perEventSourceSessionId to - // represent 0-based positive values - bool bSessionEnable = (commandArgs.perEventSourceSessionId >= 0); - if (commandArgs.perEventSourceSessionId == 0 && !commandArgs.enable) - bSessionEnable = false; + // interpret perEventSourceSessionId's sign, and adjust perEventSourceSessionId to + // represent 0-based positive values + bool bSessionEnable = (commandArgs.perEventSourceSessionId >= 0); + if (commandArgs.perEventSourceSessionId == 0 && !commandArgs.enable) + bSessionEnable = false; - if (commandArgs.listener == null) - { - if (!bSessionEnable) - commandArgs.perEventSourceSessionId = -commandArgs.perEventSourceSessionId; - // for "global" enable/disable (passed in with listener == null and - // perEventSourceSessionId == 0) perEventSourceSessionId becomes -1 - --commandArgs.perEventSourceSessionId; - } + if (commandArgs.listener == null) + { + if (!bSessionEnable) + commandArgs.perEventSourceSessionId = -commandArgs.perEventSourceSessionId; + // for "global" enable/disable (passed in with listener == null and + // perEventSourceSessionId == 0) perEventSourceSessionId becomes -1 + --commandArgs.perEventSourceSessionId; + } - commandArgs.Command = bSessionEnable ? EventCommand.Enable : EventCommand.Disable; + commandArgs.Command = bSessionEnable ? EventCommand.Enable : EventCommand.Disable; - // perEventSourceSessionId = -1 when ETW sent a notification, but the set of active sessions - // hasn't changed. - // sesisonId = SessionMask.MAX when one of the legacy ETW sessions changed - // 0 <= perEventSourceSessionId < SessionMask.MAX for activity-tracing aware sessions - Debug.Assert(commandArgs.perEventSourceSessionId >= -1 && commandArgs.perEventSourceSessionId <= SessionMask.MAX); + // perEventSourceSessionId = -1 when ETW sent a notification, but the set of active sessions + // hasn't changed. + // sesisonId = SessionMask.MAX when one of the legacy ETW sessions changed + // 0 <= perEventSourceSessionId < SessionMask.MAX for activity-tracing aware sessions + Debug.Assert(commandArgs.perEventSourceSessionId >= -1 && commandArgs.perEventSourceSessionId <= SessionMask.MAX); - // Send the manifest if we are enabling an ETW session - if (bSessionEnable && commandArgs.dispatcher == null) - { - // eventSourceDispatcher == null means this is the ETW manifest + // Send the manifest if we are enabling an ETW session + if (bSessionEnable && commandArgs.dispatcher == null) + { + // eventSourceDispatcher == null means this is the ETW manifest - // Note that we unconditionally send the manifest whenever we are enabled, even if - // we were already enabled. This is because there may be multiple sessions active - // and we can't know that all the sessions have seen the manifest. - if (!SelfDescribingEvents) - SendManifest(m_rawManifest); - } + // Note that we unconditionally send the manifest whenever we are enabled, even if + // we were already enabled. This is because there may be multiple sessions active + // and we can't know that all the sessions have seen the manifest. + if (!SelfDescribingEvents) + SendManifest(m_rawManifest); + } - // Turn on the enable bit before making the OnEventCommand callback This allows you to do useful - // things like log messages, or test if keywords are enabled in the callback. - if (commandArgs.enable) - { - Debug.Assert(m_eventData != null); - m_eventSourceEnabled = true; - } + // Turn on the enable bit before making the OnEventCommand callback This allows you to do useful + // things like log messages, or test if keywords are enabled in the callback. + if (commandArgs.enable) + { + Debug.Assert(m_eventData != null); + m_eventSourceEnabled = true; + } - this.OnEventCommand(commandArgs); - this.m_eventCommandExecuted?.Invoke(this, commandArgs); + this.OnEventCommand(commandArgs); + this.m_eventCommandExecuted?.Invoke(this, commandArgs); - if (!commandArgs.enable) - { - // If we are disabling, maybe we can turn on 'quick checks' to filter - // quickly. These are all just optimizations (since later checks will still filter) + if (!commandArgs.enable) + { + // If we are disabling, maybe we can turn on 'quick checks' to filter + // quickly. These are all just optimizations (since later checks will still filter) - // There is a good chance EnabledForAnyListener are not as accurate as - // they could be, go ahead and get a better estimate. - for (int i = 0; i < m_eventData.Length; i++) + // There is a good chance EnabledForAnyListener are not as accurate as + // they could be, go ahead and get a better estimate. + for (int i = 0; i < m_eventData.Length; i++) + { + bool isEnabledForAnyListener = false; + for (EventDispatcher? dispatcher = m_Dispatchers; dispatcher != null; dispatcher = dispatcher.m_Next) { - bool isEnabledForAnyListener = false; - for (EventDispatcher? dispatcher = m_Dispatchers; dispatcher != null; dispatcher = dispatcher.m_Next) - { - Debug.Assert(dispatcher.m_EventEnabled != null); + Debug.Assert(dispatcher.m_EventEnabled != null); - if (dispatcher.m_EventEnabled[i]) - { - isEnabledForAnyListener = true; - break; - } + if (dispatcher.m_EventEnabled[i]) + { + isEnabledForAnyListener = true; + break; } - m_eventData[i].EnabledForAnyListener = isEnabledForAnyListener; - } - - // If no events are enabled, disable the global enabled bit. - if (!AnyEventEnabled()) - { - m_level = 0; - m_matchAnyKeyword = 0; - m_eventSourceEnabled = false; } + m_eventData[i].EnabledForAnyListener = isEnabledForAnyListener; } - } - else - { - if (commandArgs.Command == EventCommand.SendManifest) + + // If no events are enabled, disable the global enabled bit. + if (!AnyEventEnabled()) { - // TODO: should we generate the manifest here if we hadn't already? - if (m_rawManifest != null) - SendManifest(m_rawManifest); + m_level = 0; + m_matchAnyKeyword = 0; + m_eventSourceEnabled = false; } - - // These are not used for non-update commands and thus should always be 'default' values - // Debug.Assert(enable == true); - // Debug.Assert(level == EventLevel.LogAlways); - // Debug.Assert(matchAnyKeyword == EventKeywords.None); - - this.OnEventCommand(commandArgs); - m_eventCommandExecuted?.Invoke(this, commandArgs); } } - catch (Exception e) + else { - // When the ETW session is created after the EventSource has registered with the ETW system - // we can send any error messages here. - ReportOutOfBandMessage("ERROR: Exception in Command Processing for EventSource " + Name + ": " + e.Message); - // We never throw when doing a command. + if (commandArgs.Command == EventCommand.SendManifest) + { + // TODO: should we generate the manifest here if we hadn't already? + if (m_rawManifest != null) + SendManifest(m_rawManifest); + } + + // These are not used for non-update commands and thus should always be 'default' values + // Debug.Assert(enable == true); + // Debug.Assert(level == EventLevel.LogAlways); + // Debug.Assert(matchAnyKeyword == EventKeywords.None); + + this.OnEventCommand(commandArgs); + m_eventCommandExecuted?.Invoke(this, commandArgs); } } + catch (Exception e) + { + // When the ETW session is created after the EventSource has registered with the ETW system + // we can send any error messages here. + ReportOutOfBandMessage("ERROR: Exception in Command Processing for EventSource " + Name + ": " + e.Message); + // We never throw when doing a command. + } } /// @@ -3831,7 +3847,7 @@ private bool SelfDescribingEvents // used for generating GUID from eventsource name private static byte[]? namespaceBytes; #endif - #endregion +#endregion } /// @@ -4125,7 +4141,7 @@ protected internal virtual void OnEventWritten(EventWrittenEventArgs eventData) this.EventWritten?.Invoke(this, eventData); } - #region private +#region private /// /// This routine adds newEventSource to the global list of eventSources, it also assigns the /// ID to the eventSource (which is simply the ordinal in the global list). @@ -4450,7 +4466,7 @@ private void CallBackForExistingEventSources(bool addToListenersList, EventHandl /// Used to register AD/Process shutdown callbacks. /// private static bool s_EventSourceShutdownRegistered = false; - #endregion +#endregion } /// @@ -4492,7 +4508,7 @@ public bool DisableEvent(int eventId) return eventSource.EnableEventForDispatcher(dispatcher, eventProviderType, eventId, false); } - #region private +#region private internal EventCommandEventArgs(EventCommand command, IDictionary? arguments, EventSource eventSource, EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId, int etwSessionId, bool enable, EventLevel level, EventKeywords matchAnyKeyword) @@ -4522,7 +4538,7 @@ internal EventCommandEventArgs(EventCommand command, IDictionary @@ -4793,7 +4809,7 @@ public DateTime TimeStamp internal set; } - #region private +#region private internal EventWrittenEventArgs(EventSource eventSource) { m_eventSource = eventSource; @@ -4809,7 +4825,7 @@ internal EventWrittenEventArgs(EventSource eventSource) internal EventOpcode m_opcode; internal EventLevel m_level; internal EventKeywords m_keywords; - #endregion +#endregion } /// @@ -4912,10 +4928,10 @@ public EventOpcode Opcode /// public EventActivityOptions ActivityOptions { get; set; } - #region private +#region private private EventOpcode m_opcode; private bool m_opcodeSet; - #endregion +#endregion } /// @@ -5058,7 +5074,7 @@ public enum EventCommand Disable = -3 } - #region private classes +#region private classes // holds a bitfield representing a session mask /// @@ -5733,7 +5749,7 @@ private string CreateManifestString() return sb.ToString(); } - #region private +#region private private void WriteNameAndMessageAttribs(StringBuilder stringBuilder, string elementName, string name) { stringBuilder.Append(" name=\"").Append(name).Append('"'); @@ -6112,7 +6128,7 @@ private class ChannelInfo private string? eventName; // Name of the event currently being processed. private int numParams; // keeps track of the number of args the event has. private List? byteArrArgIndices; // keeps track of the index of each byte[] argument - #endregion +#endregion } /// @@ -6136,5 +6152,5 @@ public enum ManifestFormats : byte #endif } - #endregion +#endregion } From bbd26fc4457f33993b1faaddde10275ceceebb0f Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 23 Jun 2020 08:34:08 -0500 Subject: [PATCH 3/5] Fix unintended changes --- .../src/System/Diagnostics/Tracing/EventSource.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 b9e9dd378993ca..9853437a153e33 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 @@ -405,7 +405,7 @@ public static string GetName(Type eventSourceType) /// public static IEnumerable GetSources() { - if (IsSupported) + if (!IsSupported) { return Array.Empty(); } @@ -1779,7 +1779,7 @@ private static Guid GenerateGuidFromName(string name) Debug.Assert(m_eventData != null); Type dataType = GetDataType(m_eventData[eventId], parameterId); - Again: + Again: if (dataType == typeof(IntPtr)) { return *((IntPtr*)dataPointer); @@ -2903,7 +2903,7 @@ private unsafe void SendManifest(byte[]? rawManifest) dataDescrs[1].Reserved = 0; int chunkSize = ManifestEnvelope.MaxChunkSize; - TRY_AGAIN_WITH_SMALLER_CHUNK_SIZE: + TRY_AGAIN_WITH_SMALLER_CHUNK_SIZE: envelope.TotalChunks = (ushort)((dataLeft + (chunkSize - 1)) / chunkSize); while (dataLeft > 0) { @@ -3442,7 +3442,7 @@ private static void AddProviderEnumKind(ManifestBuilder manifest, FieldInfo stat } #endif return; - Error: + Error: manifest.ManifestError(SR.Format(SR.EventSource_EnumKindMismatch, staticField.Name, staticField.FieldType.Name, providerEnumKind)); } From 1e198848b77f722f29b54b5ad312c7789fe6f93b Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 23 Jun 2020 16:32:11 -0500 Subject: [PATCH 4/5] Add test when EventSource IsSupported feature switch is disabled. --- .../BasicEventSourceTest/TestNotSupported.cs | 53 +++++++++++++++++++ .../System.Diagnostics.Tracing.Tests.csproj | 1 + 2 files changed, 54 insertions(+) create mode 100644 src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs diff --git a/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs new file mode 100644 index 00000000000000..0f2059d40739e9 --- /dev/null +++ b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections.Generic; +using Xunit; +using SdtEventSources; +using Microsoft.DotNet.RemoteExecutor; +#if USE_MDT_EVENTSOURCE +using Microsoft.Diagnostics.Tracing; +#else +using System.Diagnostics.Tracing; +#endif + +namespace BasicEventSourceTests +{ + public class TestNotSupported + { + /// + /// Tests using EventSource when the System.Diagnostics.Tracing.EventSource.IsSupported + /// feature switch is set to disable all EventSource operations. + /// + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [ActiveIssue("https://github.com/dotnet/arcade/pull/5698", TargetFrameworkMonikers.Any)] + public void TestBasicOperations_IsSupported_False() + { + RemoteInvokeOptions options = new RemoteInvokeOptions(); + // Requires https://github.com/dotnet/arcade/pull/5698 to be merged into dotnet/runtime + // options.RuntimeConfigurationOptions.Add("System.Diagnostics.Tracing.EventSource.IsSupported", "false"); + + RemoteExecutor.Invoke(() => + { + using (var log = new EventSourceTest()) + { + using (var listener = new LoudListener(log)) + { + IEnumerable sources = EventSource.GetSources(); + Assert.Empty(sources); + + Assert.Null(EventSource.GenerateManifest(typeof(SimpleEventSource), string.Empty, EventManifestOptions.OnlyIfNeededForRegistration)); + Assert.Null(EventSource.GenerateManifest(typeof(EventSourceTest), string.Empty, EventManifestOptions.AllCultures)); + + log.Event0(); + Assert.Null(LoudListener.LastEvent); + + EventSource.SendCommand(log, EventCommand.Enable, commandArguments: null); + Assert.False(log.IsEnabled()); + } + } + }, options).Dispose(); + } + } +} diff --git a/src/libraries/System.Diagnostics.Tracing/tests/System.Diagnostics.Tracing.Tests.csproj b/src/libraries/System.Diagnostics.Tracing/tests/System.Diagnostics.Tracing.Tests.csproj index 35c33d0095d48d..29df8cd940d035 100644 --- a/src/libraries/System.Diagnostics.Tracing/tests/System.Diagnostics.Tracing.Tests.csproj +++ b/src/libraries/System.Diagnostics.Tracing/tests/System.Diagnostics.Tracing.Tests.csproj @@ -22,6 +22,7 @@ + From 395887f5c90f56dc2e9ec953161497ce5d9a656a Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Fri, 26 Jun 2020 10:04:43 -0500 Subject: [PATCH 5/5] Update RemoteExecutor version and enable EventSource test --- eng/Version.Details.xml | 4 ++-- eng/Versions.props | 2 +- .../tests/BasicEventSourceTest/TestNotSupported.cs | 4 +--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 442156328906d3..263da61086f5b5 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -46,9 +46,9 @@ https://github.com/dotnet/arcade 71b580038fb704df63e03c6b7ae7d2c6a4fdd71d - + https://github.com/dotnet/arcade - 71b580038fb704df63e03c6b7ae7d2c6a4fdd71d + b91c34a908efe4d95ac487a33b88521c76c6c3cf https://github.com/dotnet/arcade diff --git a/eng/Versions.props b/eng/Versions.props index 9cf7f5a06a922d..406d1dacad9f20 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -59,7 +59,7 @@ 5.0.0-beta.20316.1 2.5.1-beta.20316.1 5.0.0-beta.20316.1 - 5.0.0-beta.20316.1 + 5.0.0-beta.20325.16 5.0.0-beta.20316.1 5.0.0-preview.4.20202.18 diff --git a/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs index 0f2059d40739e9..01338e8aea8c7d 100644 --- a/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs +++ b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestNotSupported.cs @@ -21,12 +21,10 @@ public class TestNotSupported /// feature switch is set to disable all EventSource operations. /// [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [ActiveIssue("https://github.com/dotnet/arcade/pull/5698", TargetFrameworkMonikers.Any)] public void TestBasicOperations_IsSupported_False() { RemoteInvokeOptions options = new RemoteInvokeOptions(); - // Requires https://github.com/dotnet/arcade/pull/5698 to be merged into dotnet/runtime - // options.RuntimeConfigurationOptions.Add("System.Diagnostics.Tracing.EventSource.IsSupported", "false"); + options.RuntimeConfigurationOptions.Add("System.Diagnostics.Tracing.EventSource.IsSupported", false); RemoteExecutor.Invoke(() => {