Move away from exposing internal EventPipe wait handle to EventPipeEventDispatcher. (#68320)

* Move away from exposing internal EventPipe wait handle to EventPipeEventDispatcher.

Fix for issue, #68032.

EventPipe exposed and internal wait handle to EventPipeEventDispatcher.
The wait handle is not a managed WaitableObject, but maps to runtime
specific event types:

CLREventStatic * on CoreCLR
mono_w32event_create on Mono

Overtime it appears that the underlying WaitHandle changed on none Windows
platforms, so calls like:

EventWaitHandle.Set

Assumes that IntPtr used is a WaitableObject and on Mono this will cause
a crash, but on CoreCLR, it will just be casted over to a WaitableObject
and use it, so based on memory layout this could cause issue, but looking
at the implementation, it appears it will always assume EventWaitHandle
is signaled so won't touch any of the memory (but not signal the event).

There have been an ambition in DispatchEventsToEventListeners to move
this abstraction down into native code, get away from using a handle
returned from EventPipeInternal.GetWaitHandle as a EventWaitHandle. This
PR eliminate EventPipeInternal.GetWaitHandle and adds a SignalSession and
WaitForSessionSignal icall's (Mono), qcall's (CoreCLR) and fix the
issue caused by exposing the handle to managed code.

Author: lateralusX

src/coreclr/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipe.CoreCLR.cs

@@ -57,8 +57,13 @@ private static unsafe partial ulong Enable(
         [return: MarshalAs(UnmanagedType.Bool)]
         internal static unsafe partial bool GetNextEvent(ulong sessionID, EventPipeEventInstanceData* pInstance);
 
-        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "EventPipeInternal_GetWaitHandle")]
-        internal static unsafe partial IntPtr GetWaitHandle(ulong sessionID);
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "EventPipeInternal_SignalSession")]
+        [return: MarshalAs(UnmanagedType.Bool)]
+        internal static unsafe partial bool SignalSession(ulong sessionID);
+
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "EventPipeInternal_WaitForSessionSignal")]
+        [return: MarshalAs(UnmanagedType.Bool)]
+        internal static unsafe partial bool WaitForSessionSignal(ulong sessionID, int timeoutMs);
     }
 }
 

src/coreclr/vm/eventpipeadapter.h

@@ -261,10 +261,26 @@ class EventPipeAdapter final
 		return ep_get_session(id);
 	}
 
-	static inline HANDLE GetWaitHandle(EventPipeSessionID id)
+	static inline bool SignalSession(EventPipeSessionID id)
 	{
 		STATIC_CONTRACT_NOTHROW;
-		return reinterpret_cast<HANDLE>(ep_get_wait_handle(id));
+
+		EventPipeSession *const session = ep_get_session (id);
+		if (!session)
+			return false;
+
+		return ep_rt_wait_event_set (ep_session_get_wait_event (session));
+	}
+
+	static inline bool WaitForSessionSignal(EventPipeSessionID id, INT32 timeoutMs)
+	{
+		STATIC_CONTRACT_NOTHROW;
+
+		EventPipeSession *const session = ep_get_session (id);
+		if (!session)
+			return false;
+
+		return !ep_rt_wait_event_wait (ep_session_get_wait_event (session), (uint32_t)timeoutMs, false) ? true : false;
 	}
 
 	static inline FILETIME GetSessionStartTime(EventPipeSession *session)

src/coreclr/vm/eventpipeinternal.cpp

@@ -255,17 +255,30 @@ extern "C" bool QCALLTYPE EventPipeInternal_GetNextEvent(UINT64 sessionID, Event
     return pNextInstance != NULL;
 }
 
-extern "C" HANDLE QCALLTYPE EventPipeInternal_GetWaitHandle(UINT64 sessionID)
+extern "C" bool QCALLTYPE EventPipeInternal_SignalSession(UINT64 sessionID)
 {
     QCALL_CONTRACT;
 
-    HANDLE waitHandle = NULL;
+    bool result = false;
     BEGIN_QCALL;
 
-    waitHandle = EventPipeAdapter::GetWaitHandle(sessionID);
+    result = EventPipeAdapter::SignalSession(sessionID);
 
     END_QCALL;
-    return waitHandle;
+    return result;
+}
+
+extern "C" bool QCALLTYPE EventPipeInternal_WaitForSessionSignal(UINT64 sessionID, INT32 timeoutMs)
+{
+    QCALL_CONTRACT;
+
+    bool result = false;
+    BEGIN_QCALL;
+
+    result = EventPipeAdapter::WaitForSessionSignal(sessionID, timeoutMs);
+
+    END_QCALL;
+    return result;
 }
 
 #endif // FEATURE_PERFTRACING

src/coreclr/vm/eventpipeinternal.h

@@ -73,7 +73,7 @@ extern "C" void QCALLTYPE EventPipeInternal_DeleteProvider(
     INT_PTR provHandle);
 
 extern "C" int QCALLTYPE EventPipeInternal_EventActivityIdControl(
-    uint32_t controlCode,
+    UINT32 controlCode,
     GUID *pActivityId);
 
 extern "C" void QCALLTYPE EventPipeInternal_WriteEventData(
@@ -86,9 +86,13 @@ extern "C" bool QCALLTYPE EventPipeInternal_GetNextEvent(
     UINT64 sessionID,
     EventPipeEventInstanceData *pInstance);
 
-extern "C" HANDLE QCALLTYPE EventPipeInternal_GetWaitHandle(
+extern "C" bool QCALLTYPE EventPipeInternal_SignalSession(
     UINT64 sessionID);
 
+extern "C" bool QCALLTYPE EventPipeInternal_WaitForSessionSignal(
+    UINT64 sessionID,
+    INT32 timeoutMs);
+
 #endif // FEATURE_PERFTRACING
 
 #endif // __EVENTPIPEINTERNAL_H__

src/coreclr/vm/qcallentrypoints.cpp

@@ -291,7 +291,8 @@ static const Entry s_QCall[] =
     DllImportEntry(EventPipeInternal_GetProvider)
     DllImportEntry(EventPipeInternal_WriteEventData)
     DllImportEntry(EventPipeInternal_GetNextEvent)
-    DllImportEntry(EventPipeInternal_GetWaitHandle)
+    DllImportEntry(EventPipeInternal_SignalSession)
+    DllImportEntry(EventPipeInternal_WaitForSessionSignal)
 #endif
 #if defined(TARGET_UNIX)
     DllImportEntry(CloseHandle)

src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventDispatcher.cs

@@ -32,7 +32,6 @@ internal EventListenerSubscription(EventKeywords matchAnyKeywords, EventLevel le
         private long m_timeQPCFrequency;
 
         private bool m_stopDispatchTask;
-        private readonly EventPipeWaitHandle m_dispatchTaskWaitHandle = new EventPipeWaitHandle();
         private Task? m_dispatchTask;
         private readonly object m_dispatchControlLock = new object();
         private readonly Dictionary<EventListener, EventListenerSubscription> m_subscriptions = new Dictionary<EventListener, EventListenerSubscription>();
@@ -43,7 +42,6 @@ private EventPipeEventDispatcher()
         {
             // Get the ID of the runtime provider so that it can be used as a filter when processing events.
             m_RuntimeProviderID = EventPipeInternal.GetProvider(NativeRuntimeEventSource.EventSourceName);
-            m_dispatchTaskWaitHandle.SafeWaitHandle = new SafeWaitHandle(IntPtr.Zero, false);
         }
 
         internal void SendCommand(EventListener eventListener, EventCommand command, bool enable, EventLevel level, EventKeywords matchAnyKeywords)
@@ -139,9 +137,6 @@ private void StartDispatchTask()
             if (m_dispatchTask == null)
             {
                 m_stopDispatchTask = false;
-                // Create a SafeWaitHandle that won't release the handle when done
-                m_dispatchTaskWaitHandle.SafeWaitHandle = new SafeWaitHandle(EventPipeInternal.GetWaitHandle(m_sessionID), false);
-
                 m_dispatchTask = Task.Factory.StartNew(DispatchEventsToEventListeners, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default);
             }
         }
@@ -153,8 +148,7 @@ private void StopDispatchTask()
             if (m_dispatchTask != null)
             {
                 m_stopDispatchTask = true;
-                Debug.Assert(!m_dispatchTaskWaitHandle.SafeWaitHandle.IsInvalid);
-                EventWaitHandle.Set(m_dispatchTaskWaitHandle.SafeWaitHandle);
+                EventPipeInternal.SignalSession(m_sessionID);
                 m_dispatchTask.Wait();
                 m_dispatchTask = null;
             }
@@ -188,9 +182,7 @@ private unsafe void DispatchEventsToEventListeners()
                 {
                     if (!eventsReceived)
                     {
-                        // Future TODO: this would make more sense to handle in EventPipeSession/EventPipe native code.
-                        Debug.Assert(!m_dispatchTaskWaitHandle.SafeWaitHandle.IsInvalid);
-                        m_dispatchTaskWaitHandle.WaitOne();
+                        EventPipeInternal.WaitForSessionSignal(m_sessionID, Timeout.Infinite);
                     }
 
                     Thread.Sleep(10);

src/mono/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.Mono.cs

@@ -57,7 +57,10 @@ internal static unsafe IntPtr GetProvider(string providerName)
         internal static extern unsafe bool GetNextEvent(ulong sessionID, EventPipeEventInstanceData* pInstance);
 
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        internal static extern unsafe IntPtr GetWaitHandle(ulong sessionID);
+        internal static extern unsafe bool SignalSession(ulong sessionID);
+
+        [MethodImplAttribute(MethodImplOptions.InternalCall)]
+        internal static extern unsafe bool WaitForSessionSignal(ulong sessionID, int timeoutMs);
 #endif // FEATURE_PERFTRACING
 
         //

src/mono/mono/component/event_pipe-stub.c

@@ -177,6 +177,14 @@ event_pipe_stub_write_event_threadpool_io_pack (
 	intptr_t overlapped,
 	uint16_t clr_instance_id);
 
+static bool
+event_pipe_stub_signal_session (EventPipeSessionID session_id);
+
+static bool
+event_pipe_stub_wait_for_session_signal (
+	EventPipeSessionID session_id,
+	uint32_t timeout);
+
 MonoComponentEventPipe *
 component_event_pipe_stub_init (void);
 
@@ -210,7 +218,9 @@ static MonoComponentEventPipe fn_table = {
 	&event_pipe_stub_write_event_threadpool_io_enqueue,
 	&event_pipe_stub_write_event_threadpool_io_dequeue,
 	&event_pipe_stub_write_event_threadpool_working_thread_count,
-	&event_pipe_stub_write_event_threadpool_io_pack
+	&event_pipe_stub_write_event_threadpool_io_pack,
+	&event_pipe_stub_signal_session,
+	&event_pipe_stub_wait_for_session_signal
 };
 
 static bool
@@ -459,6 +469,20 @@ event_pipe_stub_write_event_threadpool_io_pack (
 	return true;
 }
 
+static bool
+event_pipe_stub_signal_session (EventPipeSessionID session_id)
+{
+	return true;
+}
+
+static bool
+event_pipe_stub_wait_for_session_signal (
+	EventPipeSessionID session_id,
+	uint32_t timeout)
+{
+	return true;
+}
+
 MonoComponentEventPipe *
 component_event_pipe_stub_init (void)
 {

src/mono/mono/component/event_pipe.c

@@ -84,6 +84,14 @@ event_pipe_thread_ctrl_activity_id(
 	uint8_t *activity_id,
 	uint32_t activity_id_len);
 
+static bool
+event_pipe_signal_session (EventPipeSessionID session_id);
+
+static bool
+event_pipe_wait_for_session_signal (
+	EventPipeSessionID session_id,
+	uint32_t timeout);
+
 static MonoComponentEventPipe fn_table = {
 	{ MONO_COMPONENT_ITF_VERSION, &event_pipe_available },
 	&ep_init,
@@ -114,7 +122,9 @@ static MonoComponentEventPipe fn_table = {
 	&ep_rt_write_event_threadpool_io_enqueue,
 	&ep_rt_write_event_threadpool_io_dequeue,
 	&ep_rt_write_event_threadpool_working_thread_count,
-	&ep_rt_write_event_threadpool_io_pack
+	&ep_rt_write_event_threadpool_io_pack,
+	&event_pipe_signal_session,
+	&event_pipe_wait_for_session_signal
 };
 
 static bool
@@ -161,7 +171,7 @@ event_pipe_enable (
 		rundown_requested,
 		stream,
 		sync_callback,
-        NULL);
+		NULL);
 
 	if (config_providers) {
 		for (int i = 0; i < providers_len; ++i) {
@@ -285,6 +295,28 @@ event_pipe_thread_ctrl_activity_id (
 	return result;
 }
 
+static bool
+event_pipe_signal_session (EventPipeSessionID session_id)
+{
+	EventPipeSession *const session = ep_get_session (session_id);
+	if (!session)
+		return false;
+
+	return ep_rt_wait_event_set (ep_session_get_wait_event (session));
+}
+
+static bool
+event_pipe_wait_for_session_signal (
+	EventPipeSessionID session_id,
+	uint32_t timeout)
+{
+	EventPipeSession *const session = ep_get_session (session_id);
+	if (!session)
+		return false;
+
+	return !ep_rt_wait_event_wait (ep_session_get_wait_event (session), timeout, false) ? true : false;
+}
+
 MonoComponentEventPipe *
 mono_component_event_pipe_init (void)
 {

src/mono/mono/component/event_pipe.h

@@ -85,6 +85,14 @@ typedef bool
 typedef ep_timestamp_t
 (*event_pipe_component_convert_100ns_ticks_to_timestamp_t_func) (int64_t ticks_100ns);
 
+typedef bool
+(*event_pipe_component_signal_session) (EventPipeSessionID session_id);
+
+typedef bool
+(*event_pipe_component_wait_for_session_signal) (
+	EventPipeSessionID session_id,
+	uint32_t timeout);
+
 /*
  * EventPipeProvider.
  */
@@ -242,6 +250,8 @@ typedef struct _MonoComponentEventPipe {
 	event_pipe_component_write_event_threadpool_io_dequeue_func write_event_threadpool_io_dequeue;
 	event_pipe_component_write_event_threadpool_working_thread_count_func write_event_threadpool_working_thread_count;
 	event_pipe_component_write_event_threadpool_io_pack_func write_event_threadpool_io_pack;
+	event_pipe_component_signal_session signal_session;
+	event_pipe_component_wait_for_session_signal wait_for_session_signal;
 } MonoComponentEventPipe;
 
 MONO_COMPONENT_EXPORT_ENTRYPOINT