diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs index 2696fa21716eba..902d89111cbe44 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs @@ -35,7 +35,9 @@ unsafe uint IEventProvider.EventRegister( // Unregister an event provider. uint IEventProvider.EventUnregister(long registrationHandle) { - EventPipeInternal.DeleteProvider(m_provHandle); + IntPtr provHandle = m_provHandle; + m_provHandle = IntPtr.Zero; + EventPipeInternal.DeleteProvider(provHandle); return 0; } @@ -82,8 +84,13 @@ int IEventProvider.EventActivityIdControl(Interop.Advapi32.ActivityControl Contr unsafe IntPtr IEventProvider.DefineEventHandle(uint eventID, string eventName, long keywords, uint eventVersion, uint level, byte *pMetadata, uint metadataLength) { - IntPtr eventHandlePtr = EventPipeInternal.DefineEvent(m_provHandle, eventID, keywords, eventVersion, level, pMetadata, metadataLength); - return eventHandlePtr; + if (m_provHandle != IntPtr.Zero) + { + IntPtr eventHandlePtr = EventPipeInternal.DefineEvent(m_provHandle, eventID, keywords, eventVersion, level, pMetadata, metadataLength); + return eventHandlePtr; + } + + return IntPtr.Zero; } // Get or set the per-thread activity ID. 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 4800a2cde47365..6ea88e9fa0ef4c 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 @@ -295,7 +295,7 @@ public bool IsEnabled(EventLevel level, EventKeywords keywords, EventChannel cha if (!IsEnabled()) return false; - if (!IsEnabledCommon(m_eventSourceEnabled, m_level, m_matchAnyKeyword, level, keywords, channel)) + if (!IsEnabledCommon(IsEnabled(), m_level, m_matchAnyKeyword, level, keywords, channel)) return false; return true; @@ -1302,6 +1302,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) { + using var writeGuard = new WriteGuard(this); if (IsEnabled()) { Debug.Assert(m_eventData != null); // You must have initialized this if you enabled the source. @@ -1448,7 +1449,7 @@ protected virtual void Dispose(bool 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) + if (IsEnabled()) { try { @@ -1457,6 +1458,12 @@ protected virtual void Dispose(bool disposing) catch { } // If it fails, simply give up. m_eventSourceEnabled = false; } + + // Wait till active writes have finished (stop waiting at 1 second) + // TODO: determine appropriate break out mechanism. Is 1 second appropriate? Is a timeout appropriate at all? + // TODO: use Volatile.Read instead? It's an int, so we shouldn't get tearing. + SpinWait.SpinUntil(() => Interlocked.CompareExchange(ref m_activeWritesCount, 0, 0) == 0, 1000); + if (m_etwProvider != null) { m_etwProvider.Dispose(); @@ -1625,7 +1632,7 @@ private unsafe void Initialize(Guid eventSourceGuid, string eventSourceName, str #if FEATURE_PERFTRACING m_eventPipeProvider = eventPipeProvider; #endif - Debug.Assert(!m_eventSourceEnabled); // We can't be enabled until we are completely initted. + Debug.Assert(!IsEnabled()); // We can't be enabled until we are completely initted. // We are logically completely initialized at this point. m_completelyInited = true; } @@ -1892,6 +1899,7 @@ private static unsafe void AssertValidString(EventData* data) #endif private unsafe void WriteEventVarargs(int eventId, Guid* childActivityID, object?[] args) { + using var writeGuard = new WriteGuard(this); if (IsEnabled()) { Debug.Assert(m_eventData != null); // You must have initialized this if you enabled the source. @@ -2625,7 +2633,7 @@ internal void DoCommand(EventCommandEventArgs commandArgs) if (commandArgs.enable) { - if (!m_eventSourceEnabled) + if (!IsEnabled()) { // EventSource turned on for the first time, simply copy the bits. m_level = commandArgs.level; @@ -3838,6 +3846,23 @@ private bool SelfDescribingEvents } } + // Helper class for guarding writes to prevent Dispose from happening while we are writing + private ref struct WriteGuard + { + private EventSource _source; + + public WriteGuard(EventSource source) + { + _source = source; + Interlocked.Increment(ref _source.m_activeWritesCount); + } + + public void Dispose() + { + Interlocked.Decrement(ref _source.m_activeWritesCount); + } + } + // private instance state private string m_name = null!; // My friendly name (privided in ctor) internal int m_id; // A small integer that is unique to this instance. @@ -3852,9 +3877,10 @@ private bool SelfDescribingEvents private bool m_eventSourceDisposed; // has Dispose been called. // Enabling bits - private bool m_eventSourceEnabled; // am I enabled (any of my events are enabled for any dispatcher) + private volatile bool m_eventSourceEnabled; // am I enabled (any of my events are enabled for any dispatcher) internal EventLevel m_level; // highest level enabled by any output dispatcher internal EventKeywords m_matchAnyKeyword; // the logical OR of all levels enabled by any output dispatcher (zero is a special case) meaning 'all keywords' + private int m_activeWritesCount; // The number of currently active calls to Write methods that have already checked m_eventSourceEnabled // Dispatching state internal volatile EventDispatcher? m_Dispatchers; // Linked list of code:EventDispatchers we write the data to (we also do ETW specially) 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 c51d731e0c61e8..f23134ca9276d3 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 @@ -365,10 +365,13 @@ private unsafe void WriteMultiMerge( Guid* childActivityID, params object?[] values) { + using var writeGuard = new WriteGuard(this); + if (!this.IsEnabled()) { return; } + byte level = (options.valuesSet & EventSourceOptions.levelSet) != 0 ? options.level : eventTypes.level; @@ -550,6 +553,7 @@ internal unsafe void WriteMultiMerge( EventData* data) { #if FEATURE_MANAGED_ETW + using var writeGuard = new WriteGuard(this); if (!this.IsEnabled()) { return; @@ -621,125 +625,129 @@ private unsafe void WriteImpl( Guid* pRelatedActivityId, TraceLoggingEventTypes eventTypes) { - try + using var writeGuard = new WriteGuard(this); + if (IsEnabled()) { - fixed (EventSourceOptions* pOptions = &options) + try { - options.Opcode = options.IsOpcodeSet ? options.Opcode : GetOpcodeWithDefault(options.Opcode, eventName); - NameInfo? nameInfo = this.UpdateDescriptor(eventName, eventTypes, ref options, out EventDescriptor descriptor); - if (nameInfo == null) + fixed (EventSourceOptions* pOptions = &options) { - return; - } - -#if FEATURE_PERFTRACING - IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_eventPipeProvider, m_eventHandleTable, descriptor, eventTypes); - Debug.Assert(eventHandle != IntPtr.Zero); -#else - IntPtr eventHandle = IntPtr.Zero; -#endif - -#if FEATURE_MANAGED_ETW - int pinCount = eventTypes.pinCount; - byte* scratch = stackalloc byte[eventTypes.scratchSize]; - EventData* descriptors = stackalloc EventData[eventTypes.dataCount + 3]; - for (int i = 0; i < eventTypes.dataCount + 3; i++) - descriptors[i] = default; - - GCHandle* pins = stackalloc GCHandle[pinCount]; - for (int i = 0; i < pinCount; i++) - pins[i] = default; - - var providerMetadata = ProviderMetadata; - fixed (byte* - pMetadata0 = providerMetadata, - pMetadata1 = nameInfo.nameMetadata, - pMetadata2 = eventTypes.typeMetadata) - { - descriptors[0].SetMetadata(pMetadata0, providerMetadata.Length, 2); - descriptors[1].SetMetadata(pMetadata1, nameInfo.nameMetadata.Length, 1); - descriptors[2].SetMetadata(pMetadata2, eventTypes.typeMetadata.Length, 1); -#endif // FEATURE_MANAGED_ETW - -#if ES_BUILD_STANDALONE - System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); -#endif - EventOpcode opcode = (EventOpcode)descriptor.Opcode; + options.Opcode = options.IsOpcodeSet ? options.Opcode : GetOpcodeWithDefault(options.Opcode, eventName); + NameInfo? nameInfo = this.UpdateDescriptor(eventName, eventTypes, ref options, out EventDescriptor descriptor); + if (nameInfo == null) + { + return; + } - Guid activityId = Guid.Empty; - Guid relatedActivityId = Guid.Empty; - if (pActivityId == null && pRelatedActivityId == null && - ((options.ActivityOptions & EventActivityOptions.Disable) == 0)) + #if FEATURE_PERFTRACING + IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_eventPipeProvider, m_eventHandleTable, descriptor, eventTypes); + Debug.Assert(eventHandle != IntPtr.Zero); + #else + IntPtr eventHandle = IntPtr.Zero; + #endif + + #if FEATURE_MANAGED_ETW + int pinCount = eventTypes.pinCount; + byte* scratch = stackalloc byte[eventTypes.scratchSize]; + EventData* descriptors = stackalloc EventData[eventTypes.dataCount + 3]; + for (int i = 0; i < eventTypes.dataCount + 3; i++) + descriptors[i] = default; + + GCHandle* pins = stackalloc GCHandle[pinCount]; + for (int i = 0; i < pinCount; i++) + pins[i] = default; + + var providerMetadata = ProviderMetadata; + fixed (byte* + pMetadata0 = providerMetadata, + pMetadata1 = nameInfo.nameMetadata, + pMetadata2 = eventTypes.typeMetadata) { - if (opcode == EventOpcode.Start) + descriptors[0].SetMetadata(pMetadata0, providerMetadata.Length, 2); + descriptors[1].SetMetadata(pMetadata1, nameInfo.nameMetadata.Length, 1); + descriptors[2].SetMetadata(pMetadata2, eventTypes.typeMetadata.Length, 1); + #endif // FEATURE_MANAGED_ETW + + #if ES_BUILD_STANDALONE + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); + #endif + EventOpcode opcode = (EventOpcode)descriptor.Opcode; + + Guid activityId = Guid.Empty; + Guid relatedActivityId = Guid.Empty; + if (pActivityId == null && pRelatedActivityId == null && + ((options.ActivityOptions & EventActivityOptions.Disable) == 0)) { - Debug.Assert(eventName != null, "GetOpcodeWithDefault should not returned Start when eventName is null"); - m_activityTracker.OnStart(m_name, eventName, 0, ref activityId, ref relatedActivityId, options.ActivityOptions); + if (opcode == EventOpcode.Start) + { + Debug.Assert(eventName != null, "GetOpcodeWithDefault should not returned Start when eventName is null"); + m_activityTracker.OnStart(m_name, eventName, 0, ref activityId, ref relatedActivityId, options.ActivityOptions); + } + else if (opcode == EventOpcode.Stop) + { + Debug.Assert(eventName != null, "GetOpcodeWithDefault should not returned Stop when eventName is null"); + m_activityTracker.OnStop(m_name, eventName, 0, ref activityId); + } + if (activityId != Guid.Empty) + pActivityId = &activityId; + if (relatedActivityId != Guid.Empty) + pRelatedActivityId = &relatedActivityId; } - else if (opcode == EventOpcode.Stop) + + try { - Debug.Assert(eventName != null, "GetOpcodeWithDefault should not returned Stop when eventName is null"); - m_activityTracker.OnStop(m_name, eventName, 0, ref activityId); + #if FEATURE_MANAGED_ETW + DataCollector.ThreadInstance.Enable( + scratch, + eventTypes.scratchSize, + descriptors + 3, + eventTypes.dataCount, + pins, + pinCount); + + TraceLoggingTypeInfo info = eventTypes.typeInfos[0]; + info.WriteData(info.PropertyValueFactory(data)); + + this.WriteEventRaw( + eventName, + ref descriptor, + eventHandle, + pActivityId, + pRelatedActivityId, + (int)(DataCollector.ThreadInstance.Finish() - descriptors), + (IntPtr)descriptors); + #endif // FEATURE_MANAGED_ETW + + // TODO enable filtering for listeners. + if (m_Dispatchers != null) + { + var eventData = (EventPayload?)(eventTypes.typeInfos[0].GetData(data)); + WriteToAllListeners(eventName, ref descriptor, nameInfo.tags, pActivityId, pRelatedActivityId, eventData); + } } - if (activityId != Guid.Empty) - pActivityId = &activityId; - if (relatedActivityId != Guid.Empty) - pRelatedActivityId = &relatedActivityId; - } - - try - { -#if FEATURE_MANAGED_ETW - DataCollector.ThreadInstance.Enable( - scratch, - eventTypes.scratchSize, - descriptors + 3, - eventTypes.dataCount, - pins, - pinCount); - - TraceLoggingTypeInfo info = eventTypes.typeInfos[0]; - info.WriteData(info.PropertyValueFactory(data)); - - this.WriteEventRaw( - eventName, - ref descriptor, - eventHandle, - pActivityId, - pRelatedActivityId, - (int)(DataCollector.ThreadInstance.Finish() - descriptors), - (IntPtr)descriptors); -#endif // FEATURE_MANAGED_ETW - - // TODO enable filtering for listeners. - if (m_Dispatchers != null) + catch (Exception ex) { - var eventData = (EventPayload?)(eventTypes.typeInfos[0].GetData(data)); - WriteToAllListeners(eventName, ref descriptor, nameInfo.tags, pActivityId, pRelatedActivityId, eventData); + if (ex is EventSourceException) + throw; + else + ThrowEventSourceException(eventName, ex); + } + #if FEATURE_MANAGED_ETW + finally + { + WriteCleanup(pins, pinCount); } } - catch (Exception ex) - { - if (ex is EventSourceException) - throw; - else - ThrowEventSourceException(eventName, ex); - } -#if FEATURE_MANAGED_ETW - finally - { - WriteCleanup(pins, pinCount); - } + #endif // FEATURE_MANAGED_ETW } -#endif // FEATURE_MANAGED_ETW } - } - catch (Exception ex) - { - if (ex is EventSourceException) - throw; - else - ThrowEventSourceException(eventName, ex); + catch (Exception ex) + { + if (ex is EventSourceException) + throw; + else + ThrowEventSourceException(eventName, ex); + } } }