From 8ad7d5e57e0eb0308453895693a996f5a93c8e08 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jos=C3=A9=20Rivero?= Date: Tue, 12 Feb 2019 13:54:33 -0800 Subject: [PATCH] Bug Fix: Calling System.Diagnostics.Tracing.EventPipe.Enable twice asserts #22247 (#22318) After disabling EventPipe -> SampleProfiler, we were not closing the thread shutdown event, thus asserting on reentrance of the EventPipe. * Adding regression test, and removing comment. * Waits until the specified object is in the signaled state. --- src/vm/sampleprofiler.cpp | 7 +- .../tracing/regress/GitHub_22247/GitHub_22247.cs | 183 +++++++++++++++++++++ .../regress/GitHub_22247/GitHub_22247.csproj | 33 ++++ 3 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 tests/src/tracing/regress/GitHub_22247/GitHub_22247.cs create mode 100644 tests/src/tracing/regress/GitHub_22247/GitHub_22247.csproj diff --git a/src/vm/sampleprofiler.cpp b/src/vm/sampleprofiler.cpp index 4548620..44ab2f8 100644 --- a/src/vm/sampleprofiler.cpp +++ b/src/vm/sampleprofiler.cpp @@ -15,7 +15,7 @@ #include #endif //FEATURE_PAL -#define NUM_NANOSECONDS_IN_1_MS (1000000) +const unsigned long NUM_NANOSECONDS_IN_1_MS = 1000000; Volatile SampleProfiler::s_profilingEnabled = false; Thread* SampleProfiler::s_pSamplingThread = NULL; @@ -48,7 +48,7 @@ void SampleProfiler::Enable() PRECONDITION(EventPipe::GetLock()->OwnedByCurrentThread()); } CONTRACTL_END; - + LoadDependencies(); if(s_pEventPipeProvider == NULL) @@ -121,7 +121,8 @@ void SampleProfiler::Disable() s_profilingEnabled = false; // Wait for the sampling thread to clean itself up. - s_threadShutdownEvent.Wait(0, FALSE /* bAlertable */); + s_threadShutdownEvent.Wait(INFINITE, FALSE /* bAlertable */); + s_threadShutdownEvent.CloseEvent(); if(s_timePeriodIsSet) { diff --git a/tests/src/tracing/regress/GitHub_22247/GitHub_22247.cs b/tests/src/tracing/regress/GitHub_22247/GitHub_22247.cs new file mode 100644 index 0000000..a35c2b7 --- /dev/null +++ b/tests/src/tracing/regress/GitHub_22247/GitHub_22247.cs @@ -0,0 +1,183 @@ +using System; +using System.Diagnostics.Tracing; +using System.Reflection; + +namespace EventPipe.Issue22247 +{ + public sealed class TraceConfiguration + { + private ConstructorInfo m_configurationCtor; + private MethodInfo m_enableProviderMethod; + private MethodInfo m_setProfilerSamplingRateMethod; + + private object m_configurationObject; + + public TraceConfiguration( + string outputFile, + uint circularBufferMB) + { + // Initialize reflection references. + if (!Initialize()) + { + throw new InvalidOperationException("Reflection failed."); + } + + m_configurationObject = m_configurationCtor.Invoke( + new object[] + { + outputFile, + circularBufferMB + }); + } + + public void EnableProvider( + string providerName, + UInt64 keywords, + uint level) + { + m_enableProviderMethod.Invoke( + m_configurationObject, + new object[] + { + providerName, + keywords, + level + }); + } + + internal object ConfigurationObject + { + get { return m_configurationObject; } + } + + public void SetSamplingRate(TimeSpan minDelayBetweenSamples) + { + m_setProfilerSamplingRateMethod.Invoke( + m_configurationObject, + new object[] + { + minDelayBetweenSamples + }); + } + + private bool Initialize() + { + Assembly SPC = typeof(System.Diagnostics.Tracing.EventSource).Assembly; + if (SPC == null) + { + Console.WriteLine("System.Private.CoreLib assembly == null"); + return false; + } + + Type configurationType = SPC.GetType("System.Diagnostics.Tracing.EventPipeConfiguration"); + if (configurationType == null) + { + Console.WriteLine("configurationType == null"); + return false; + } + + m_configurationCtor = configurationType.GetConstructor( + BindingFlags.NonPublic | BindingFlags.Instance, + null, + new Type[] { typeof(string), typeof(uint) }, + null); + if (m_configurationCtor == null) + { + Console.WriteLine("configurationCtor == null"); + return false; + } + + m_enableProviderMethod = configurationType.GetMethod( + "EnableProvider", + BindingFlags.NonPublic | BindingFlags.Instance); + if (m_enableProviderMethod == null) + { + Console.WriteLine("enableProviderMethod == null"); + return false; + } + + m_setProfilerSamplingRateMethod = configurationType.GetMethod( + "SetProfilerSamplingRate", + BindingFlags.NonPublic | BindingFlags.Instance); + if (m_setProfilerSamplingRateMethod == null) + { + Console.WriteLine("setProfilerSamplingRate == null"); + return false; + } + + return true; + } + } + + class Program + { + private static MethodInfo m_enableMethod; + private static MethodInfo m_disableMethod; + + public static void Enable(TraceConfiguration traceConfig) + { + m_enableMethod.Invoke( + null, + new object[] + { + traceConfig.ConfigurationObject + }); + } + + public static void Disable() + { + m_disableMethod.Invoke( + null, + null); + } + + static int Main(string[] args) + { + TimeSpan profSampleDelay = TimeSpan.FromMilliseconds(1); + string outputFile = "default.netperf"; + + Assembly SPC = typeof(System.Diagnostics.Tracing.EventSource).Assembly; + Type eventPipeType = SPC.GetType("System.Diagnostics.Tracing.EventPipe"); + m_enableMethod = eventPipeType.GetMethod("Enable", BindingFlags.NonPublic | BindingFlags.Static); + m_disableMethod = eventPipeType.GetMethod("Disable", BindingFlags.NonPublic | BindingFlags.Static); + + // Setup the configuration values. + uint circularBufferMB = 1024; // 1 GB + uint level = 5; // Verbose + + // Create a new instance of EventPipeConfiguration. + TraceConfiguration config = new TraceConfiguration(outputFile, circularBufferMB); + // Setup the provider values. + // Public provider. + string providerName = "Microsoft-Windows-DotNETRuntime"; + UInt64 keywords = 0x4c14fccbd; + + // Enable the provider. + config.EnableProvider(providerName, keywords, level); + + // Private provider. + providerName = "Microsoft-Windows-DotNETRuntimePrivate"; + keywords = 0x4002000b; + + // Enable the provider. + config.EnableProvider(providerName, keywords, level); + + // Sample profiler. + providerName = "Microsoft-DotNETCore-SampleProfiler"; + keywords = 0x0; + + // Enable the provider. + config.EnableProvider(providerName, keywords, level); + + // Set the sampling rate. + config.SetSamplingRate(profSampleDelay); + + // Enable tracing. + Enable(config); + Disable(); + Enable(config); + + return 100; + } + } +} diff --git a/tests/src/tracing/regress/GitHub_22247/GitHub_22247.csproj b/tests/src/tracing/regress/GitHub_22247/GitHub_22247.csproj new file mode 100644 index 0000000..ed619f2 --- /dev/null +++ b/tests/src/tracing/regress/GitHub_22247/GitHub_22247.csproj @@ -0,0 +1,33 @@ + + + + + Debug + AnyCPU + 2.0 + {8E3244CB-407F-4142-BAAB-E7A55901A5FA} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + BuildAndRun + $(DefineConstants);STATIC + true + 0 + + + true + true + + + + + + + False + + + + + + + -- 2.7.4