Bug Fix: Calling System.Diagnostics.Tracing.EventPipe.Enable twice asserts #22247...

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.

diff --git a/src/vm/sampleprofiler.cpp b/src/vm/sampleprofiler.cpp
index 4548620..44ab2f8 100644 (file)
--- a/src/vm/sampleprofiler.cpp
+++ b/src/vm/sampleprofiler.cpp
@@ -15,7 +15,7 @@
 #include <mmsystem.h>
 #endif //FEATURE_PAL
 
-#define NUM_NANOSECONDS_IN_1_MS (1000000)
+const unsigned long NUM_NANOSECONDS_IN_1_MS = 1000000;
 
 Volatile<BOOL> 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 (file)
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 (file)
index 0000000..ed619f2
--- /dev/null
+++ b/tests/src/tracing/regress/GitHub_22247/GitHub_22247.csproj
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{8E3244CB-407F-4142-BAAB-E7A55901A5FA}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <DefineConstants>$(DefineConstants);STATIC</DefineConstants>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <CLRTestPriority>0</CLRTestPriority>
+
+    <!-- Test unsupported outside of windows -->
+    <TestUnsupportedOutsideWindows>true</TestUnsupportedOutsideWindows>
+    <DisableProjectBuild Condition="'$(TargetsUnix)' == 'true'">true</DisableProjectBuild>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'"></PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x64'"></PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="GitHub_22247.cs" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>