From: David Mason Date: Fri, 24 Apr 2020 22:28:23 +0000 (-0700) Subject: Code review feedback X-Git-Tag: submit/tizen/20210909.063632~8330^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a3b30afae2b775c4c82c02398d26c84b4904e44d;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Code review feedback --- diff --git a/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs b/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs index 480a492..b8c7d9b 100644 --- a/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs +++ b/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs @@ -81,6 +81,7 @@ namespace Tracing.Tests.EventSourceError source.Dynamic.All += (TraceEvent traceEvent) => { if (traceEvent.ProviderName == "SentinelEventSource" + || traceEvent.ProviderName == "Microsoft-Windows-DotNETRuntime" || traceEvent.ProviderName == "Microsoft-Windows-DotNETRuntimeRundown" || traceEvent.ProviderName == "Microsoft-DotNETCore-EventPipe") { @@ -90,7 +91,7 @@ namespace Tracing.Tests.EventSourceError ++eventCount; if (traceEvent.ProviderName == "IllegalTypesEventSource" - && traceEvent.EventName == "WriteEventString" + && traceEvent.EventName == "EventSourceMessage" && traceEvent.FormattedMessage.StartsWith("ERROR: Exception in Command Processing for EventSource IllegalTypesEventSource", StringComparison.OrdinalIgnoreCase)) { sawEvent = true; 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 d2e33d5..34a7b86 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 @@ -8,8 +8,6 @@ namespace System.Diagnostics.Tracing { // The EventPipeProvider handle. private IntPtr m_provHandle = IntPtr.Zero; - private object m_createEventLock = new object(); - private IntPtr m_writeEventStringeEventHandle = IntPtr.Zero; // Register an event provider. unsafe uint IEventProvider.EventRegister( @@ -52,7 +50,7 @@ namespace System.Diagnostics.Tracing int userDataCount, EventProvider.EventData* userData) { - if (eventDescriptor.EventId != 0 && eventHandle != IntPtr.Zero) + if (eventHandle != IntPtr.Zero) { if (userDataCount == 0) { @@ -71,30 +69,7 @@ namespace System.Diagnostics.Tracing } EventPipeInternal.WriteEventData(eventHandle, userData, (uint)userDataCount, activityId, relatedActivityId); } - else - { - if (m_writeEventStringeEventHandle == IntPtr.Zero) - { - lock (m_createEventLock) - { - if (m_writeEventStringeEventHandle == IntPtr.Zero) - { - string eventName = "WriteEventString"; - EventParameterInfo paramInfo = default(EventParameterInfo); - paramInfo.SetInfo("message", typeof(string)); - byte[]? metadata = EventPipeMetadataGenerator.Instance.GenerateMetadata(0, eventName, 0, (uint)EventLevel.LogAlways, 0, new EventParameterInfo[] { paramInfo }); - uint metadataLength = (metadata != null) ? (uint)metadata.Length : 0; - fixed (byte* pMetadata = metadata) - { - m_writeEventStringeEventHandle = ((IEventProvider)this).DefineEventHandle(0, eventName, 0, 0, (uint)EventLevel.LogAlways, pMetadata, metadataLength); - } - } - } - } - - EventPipeInternal.WriteEventData(m_writeEventStringeEventHandle, userData, (uint)userDataCount, activityId, relatedActivityId); - } return EventProvider.WriteEventErrorCode.NoError; } 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 7ffb21c..fd42fb6 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 @@ -2149,7 +2149,11 @@ namespace System.Diagnostics.Tracing } } - private unsafe void WriteEventString(EventLevel level, long keywords, string msgString) + // WriteEventString is used for logging an error message (or similar) to + // ETW and EventPipe providers. It is not a general purpose API, it will + // log the message with Level=LogAlways and Keywords=All to make sure whoever + // is listening gets the message. + private unsafe void WriteEventString(string msgString) { #if FEATURE_MANAGED_ETW || FEATURE_PERFTRACING bool allAreNull = true; @@ -2164,6 +2168,8 @@ namespace System.Diagnostics.Tracing return; } + EventLevel level = EventLevel.LogAlways; + long keywords = -1; const string EventName = "EventSourceMessage"; if (SelfDescribingEvents) { @@ -2183,7 +2189,7 @@ namespace System.Diagnostics.Tracing if (m_rawManifest == null && m_outOfBandMessageCount == 1) { ManifestBuilder manifestBuilder = new ManifestBuilder(Name, Guid, Name, null, EventManifestOptions.None); - manifestBuilder.StartEvent(EventName, new EventAttribute(0) { Level = EventLevel.LogAlways, Task = (EventTask)0xFFFE }); + manifestBuilder.StartEvent(EventName, new EventAttribute(0) { Level = level, Task = (EventTask)0xFFFE }); manifestBuilder.AddEventParameter(typeof(string), "message"); manifestBuilder.EndEvent(); SendManifest(manifestBuilder.CreateManifest()); @@ -2206,7 +2212,27 @@ namespace System.Diagnostics.Tracing #if FEATURE_PERFTRACING if (m_eventPipeProvider != null) { - m_eventPipeProvider.WriteEvent(ref descr, IntPtr.Zero, null, null, 1, (IntPtr)((void*)&data)); + if (m_writeEventStringEventHandle == IntPtr.Zero) + { + lock (m_createEventLock) + { + if (m_writeEventStringEventHandle == IntPtr.Zero) + { + string eventName = "EventSourceMessage"; + EventParameterInfo paramInfo = default(EventParameterInfo); + paramInfo.SetInfo("message", typeof(string)); + byte[]? metadata = EventPipeMetadataGenerator.Instance.GenerateMetadata(0, eventName, keywords, (uint)level, 0, new EventParameterInfo[] { paramInfo }); + uint metadataLength = (metadata != null) ? (uint)metadata.Length : 0; + + fixed (byte* pMetadata = metadata) + { + m_writeEventStringEventHandle = m_eventPipeProvider.m_eventProvider.DefineEventHandle(0, eventName, keywords, 0, (uint)level, pMetadata, metadataLength); + } + } + } + } + + m_eventPipeProvider.WriteEvent(ref descr, m_writeEventStringEventHandle, null, null, 1, (IntPtr)((void*)&data)); } #endif // FEATURE_PERFTRACING } @@ -3824,7 +3850,7 @@ namespace System.Diagnostics.Tracing msg = "Reached message limit. End of EventSource error messages."; } - WriteEventString(EventLevel.LogAlways, -1, msg); + WriteEventString(msg); WriteStringToAllListeners("EventSourceMessage", msg); } catch { } // If we fail during last chance logging, well, we have to give up.... @@ -3881,6 +3907,8 @@ namespace System.Diagnostics.Tracing private volatile OverideEventProvider m_etwProvider = null!; // This hooks up ETW commands to our 'OnEventCommand' callback #endif #if FEATURE_PERFTRACING + private object m_createEventLock = new object(); + private IntPtr m_writeEventStringEventHandle = IntPtr.Zero; private volatile OverideEventProvider m_eventPipeProvider = null!; #endif private bool m_completelyInited; // The EventSource constructor has returned without exception.