[Mono] Fix wait's returning false on non-runtime queued user APC's. (#116001)

Mono's internal interrupt handling uses user APC to break waits in case runtime needs to interrupt threads. If that happens, wait will return false. For API's like `ManualResetEventSlim.Wait()` with `void` return, this can cause issues if the wait is broken prematurely. If a user APC have been queued to the thread, not triggered by internal runtime interrupt handling, it would lead to unexpected side effects.

Fix makes sure runtime ignores returns from WaitForSingleObjectEx/WaitForMultipleObjectsEx due to APC completions not initialized by runtime.

Fix also adds a new runtime test to trigger this scenario for Mono runtime on Windows.

Fixes #115178.

Author: lateralusX

src/mono/mono/metadata/monitor.c

@@ -1345,7 +1345,7 @@ mono_monitor_wait (MonoObjectHandle obj_handle, guint32 ms, MonoBoolean allow_in
 	 * is private to this thread.  Therefore even if the event was
 	 * signalled before we wait, we still succeed.
 	 */
-	ret = mono_w32handle_wait_one (event, ms, TRUE);
+	ret = mono_w32handle_wait_one (event, ms, allow_interruption);
 
 	/* Reset the thread state fairly early, so we don't have to worry
 	 * about the monitor error checking
@@ -1390,6 +1390,10 @@ mono_monitor_wait (MonoObjectHandle obj_handle, guint32 ms, MonoBoolean allow_in
 		 * wait queue
 		 */
 		mon->wait_list = g_slist_remove (mon->wait_list, event);
+
+		if (is_ok (error) && allow_interruption && ret == MONO_W32HANDLE_WAIT_RET_ALERTED)
+			mono_error_set_thread_interrupted (error);
+
 	}
 	mono_w32event_close (event);
 

src/mono/mono/utils/mono-os-wait-win32.c

@@ -15,6 +15,7 @@
 #include "mono-error-internals.h"
 #include <mono/utils/checked-build.h>
 #include <mono/utils/w32subset.h>
+#include <mono/utils/mono-time.h>
 
 /* Empty handler only used to detect interrupt state of current thread. */
 /* Needed in order to correctly avoid entering wait methods under */
@@ -61,22 +62,23 @@ win32_wait_interrupt_handler (gpointer ignored)
 #define WIN32_ENTER_ALERTABLE_WAIT(info) \
 	do { \
 		if (info) { \
-			gboolean alerted = FALSE; \
-			mono_thread_info_install_interrupt (win32_wait_interrupt_handler, NULL, &alerted); \
-			if (alerted) { \
+			gboolean interrupted = FALSE; \
+			mono_thread_info_install_interrupt (win32_wait_interrupt_handler, NULL, &interrupted); \
+			if (interrupted) { \
 				SetLastError (WAIT_IO_COMPLETION); \
 				return WAIT_IO_COMPLETION; \
 			} \
 			mono_win32_enter_alertable_wait (info); \
 		} \
 	} while (0)
 
-#define WIN32_LEAVE_ALERTABLE_WAIT(info) \
+#define WIN32_LEAVE_ALERTABLE_WAIT(info, alerted) \
 	do { \
 		if (info) { \
-			gboolean alerted = FALSE; \
-			mono_win32_leave_alertable_wait (info); \
-			mono_thread_info_uninstall_interrupt (&alerted); \
+			alerted = mono_win32_leave_alertable_wait (info); \
+			gboolean interrupted; \
+			mono_thread_info_uninstall_interrupt (&interrupted); \
+			alerted = alerted || interrupted; \
 		} \
 	} while (0)
 
@@ -93,17 +95,46 @@ win32_wait_for_single_object_ex (HANDLE handle, DWORD timeout, BOOL alertable, B
 	DWORD result = WAIT_FAILED;
 	MonoThreadInfo * const info = alertable ? mono_thread_info_current_unchecked () : NULL;
 
-	WIN32_ENTER_ALERTABLE_WAIT (info);
+	guint64 start_ticks = 0;
+	gboolean done = FALSE;
 
-	if (cooperative) {
-		MONO_ENTER_GC_SAFE;
-		result = win32_wait_for_single_object_ex_interrupt_checked (info, handle, timeout, alertable);
-		MONO_EXIT_GC_SAFE;
-	} else {
-		result = win32_wait_for_single_object_ex_interrupt_checked (info, handle, timeout, alertable);
-	}
+	if (timeout != INFINITE && alertable)
+		start_ticks = GINT64_TO_UINT64 (mono_msec_ticks ());
+
+	while (!done)
+	{
+		DWORD current_timeout = timeout;
+
+		if (timeout != INFINITE && alertable && result == WAIT_IO_COMPLETION) {
+			DWORD elapsed = 0;
+			guint64 current_ticks = mono_msec_ticks ();
+
+			if (current_ticks >= start_ticks)
+				elapsed = GUINT64_TO_UINT32 (current_ticks - start_ticks);
+			else
+				elapsed = GUINT64_TO_UINT32 (G_MAXUINT64 - start_ticks + current_ticks);
+
+			if (elapsed > timeout)
+				return WAIT_TIMEOUT;
+
+			current_timeout = timeout - elapsed;
+		}
+
+		WIN32_ENTER_ALERTABLE_WAIT (info);
+
+		if (cooperative) {
+			MONO_ENTER_GC_SAFE;
+			result = win32_wait_for_single_object_ex_interrupt_checked (info, handle, current_timeout, alertable);
+			MONO_EXIT_GC_SAFE;
+		} else {
+			result = win32_wait_for_single_object_ex_interrupt_checked (info, handle, current_timeout, alertable);
+		}
+
+		gboolean alerted = FALSE;
+		WIN32_LEAVE_ALERTABLE_WAIT (info, alerted);
 
-	WIN32_LEAVE_ALERTABLE_WAIT (info);
+		done = !(alertable && !alerted && result == WAIT_IO_COMPLETION);
+	}
 
 	return result;
 }
@@ -133,17 +164,46 @@ win32_wait_for_multiple_objects_ex (DWORD count, CONST HANDLE *handles, BOOL wai
 	DWORD result = WAIT_FAILED;
 	MonoThreadInfo * const info = alertable ? mono_thread_info_current_unchecked () : NULL;
 
-	WIN32_ENTER_ALERTABLE_WAIT (info);
+	guint64 start_ticks = 0;
+	gboolean done = FALSE;
 
-	if (cooperative) {
-		MONO_ENTER_GC_SAFE;
-		result = win32_wait_for_multiple_objects_ex_interrupt_checked (info, count, handles, waitAll, timeout, alertable);
-		MONO_EXIT_GC_SAFE;
-	} else {
-		result = win32_wait_for_multiple_objects_ex_interrupt_checked (info, count, handles, waitAll, timeout, alertable);
-	}
+	if (timeout != INFINITE && alertable)
+		start_ticks = GINT64_TO_UINT64 (mono_msec_ticks ());
+
+	while (!done)
+	{
+		DWORD current_timeout = timeout;
+
+		if (timeout != INFINITE && alertable && result == WAIT_IO_COMPLETION) {
+			DWORD elapsed = 0;
+			guint64 current_ticks = mono_msec_ticks ();
+
+			if (current_ticks >= start_ticks)
+				elapsed = GUINT64_TO_UINT32 (current_ticks - start_ticks);
+			else
+				elapsed = GUINT64_TO_UINT32 (G_MAXUINT64 - start_ticks + current_ticks);
+
+			if (elapsed > timeout)
+				return WAIT_TIMEOUT;
+
+			current_timeout = timeout - elapsed;
+		}
+
+		WIN32_ENTER_ALERTABLE_WAIT (info);
+
+		if (cooperative) {
+			MONO_ENTER_GC_SAFE;
+			result = win32_wait_for_multiple_objects_ex_interrupt_checked (info, count, handles, waitAll, current_timeout, alertable);
+			MONO_EXIT_GC_SAFE;
+		} else {
+			result = win32_wait_for_multiple_objects_ex_interrupt_checked (info, count, handles, waitAll, current_timeout, alertable);
+		}
+
+		gboolean alerted = FALSE;
+		WIN32_LEAVE_ALERTABLE_WAIT (info, alerted);
 
-	WIN32_LEAVE_ALERTABLE_WAIT (info);
+		done = !(alertable && !alerted && result == WAIT_IO_COMPLETION);
+	}
 
 	// This is not perfect, but it is the best you can do in usermode and matches CoreCLR.
 	// i.e. handle-based instead of object-based.

src/mono/mono/utils/mono-threads-windows.c

@@ -131,16 +131,22 @@ enter_alertable_wait_ex (MonoThreadInfo *info, HANDLE io_handle)
 	mono_atomic_xchg_i32 (&info->win32_apc_info, (io_handle == INVALID_HANDLE_VALUE) ? WIN32_APC_INFO_ALERTABLE_WAIT_SLOT : WIN32_APC_INFO_BLOCKING_IO_SLOT);
 }
 
-static void
+static gboolean
 leave_alertable_wait_ex (MonoThreadInfo *info, HANDLE io_handle)
 {
 	// Clear any previous flags. Thread is exiting alertable wait region, and info around pending interrupt/abort APC's
 	// can now be discarded, thread is out of wait operation and can proceed execution.
-	mono_atomic_xchg_i32 (&info->win32_apc_info, WIN32_APC_INFO_CLEARED);
+	gint32 old = mono_atomic_xchg_i32 (&info->win32_apc_info, WIN32_APC_INFO_CLEARED);
 
 	// Only loaded/stored by current thread, here or in APC (also running on current thread).
 	g_assert (info->win32_apc_info_io_handle == io_handle);
 	info->win32_apc_info_io_handle = (gpointer)INVALID_HANDLE_VALUE;
+
+	gboolean alerted = FALSE;
+	if (old & WIN32_APC_INFO_PENDING_INTERRUPT_SLOT || old & WIN32_APC_INFO_PENDING_ABORT_SLOT)
+		alerted = TRUE;
+
+	return alerted;
 }
 
 void
@@ -150,11 +156,14 @@ mono_win32_enter_alertable_wait (THREAD_INFO_TYPE *info)
 		enter_alertable_wait_ex (info, INVALID_HANDLE_VALUE);
 }
 
-void
+gboolean
 mono_win32_leave_alertable_wait (THREAD_INFO_TYPE *info)
 {
+	gboolean alerted = FALSE;
 	if (info)
-		leave_alertable_wait_ex (info, INVALID_HANDLE_VALUE);
+		alerted = leave_alertable_wait_ex (info, INVALID_HANDLE_VALUE);
+
+	return alerted;
 }
 
 void
@@ -164,11 +173,14 @@ mono_win32_enter_blocking_io_call (THREAD_INFO_TYPE *info, HANDLE io_handle)
 		enter_alertable_wait_ex (info, io_handle);
 }
 
-void
+gboolean
 mono_win32_leave_blocking_io_call (THREAD_INFO_TYPE *info, HANDLE io_handle)
 {
+	gboolean alerted = FALSE;
 	if (info)
-		leave_alertable_wait_ex (info, io_handle);
+		alerted = leave_alertable_wait_ex (info, io_handle);
+
+	return alerted;
 }
 
 void

src/mono/mono/utils/mono-threads.h

@@ -859,13 +859,13 @@ void mono_target_thread_schedule_synchronization_context(MonoNativeThreadId targ
 void
 mono_win32_enter_alertable_wait (THREAD_INFO_TYPE *info);
 
-void
+gboolean
 mono_win32_leave_alertable_wait (THREAD_INFO_TYPE *info);
 
 void
 mono_win32_enter_blocking_io_call (THREAD_INFO_TYPE *info, HANDLE io_handle);
 
-void
+gboolean
 mono_win32_leave_blocking_io_call (THREAD_INFO_TYPE *info, HANDLE io_handle);
 
 void