diff --git a/src/mono/mono/metadata/monitor.c b/src/mono/mono/metadata/monitor.c index 2e08f94d3361e7..acc55ff28f7f89 100644 --- a/src/mono/mono/metadata/monitor.c +++ b/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); diff --git a/src/mono/mono/utils/mono-os-wait-win32.c b/src/mono/mono/utils/mono-os-wait-win32.c index abb75cc07e3750..857f9ac8c10513 100644 --- a/src/mono/mono/utils/mono-os-wait-win32.c +++ b/src/mono/mono/utils/mono-os-wait-win32.c @@ -15,6 +15,7 @@ #include "mono-error-internals.h" #include #include +#include /* Empty handler only used to detect interrupt state of current thread. */ /* Needed in order to correctly avoid entering wait methods under */ @@ -61,9 +62,9 @@ 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; \ } \ @@ -71,12 +72,13 @@ win32_wait_interrupt_handler (gpointer ignored) } \ } 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. diff --git a/src/mono/mono/utils/mono-threads-windows.c b/src/mono/mono/utils/mono-threads-windows.c index 3e56205c0ab880..21997a97b624c1 100644 --- a/src/mono/mono/utils/mono-threads-windows.c +++ b/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 diff --git a/src/mono/mono/utils/mono-threads.h b/src/mono/mono/utils/mono-threads.h index 0706523a1eab41..44f2535fb3c45c 100644 --- a/src/mono/mono/utils/mono-threads.h +++ b/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 diff --git a/src/tests/baseservices/threading/regressions/115178/115178.cs b/src/tests/baseservices/threading/regressions/115178/115178.cs new file mode 100644 index 00000000000000..0ad7a004945eab --- /dev/null +++ b/src/tests/baseservices/threading/regressions/115178/115178.cs @@ -0,0 +1,294 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; +using System.Diagnostics; +using System.Threading; +using Xunit; + +/* + * Issue description: + User APCs can be queued to a thread that is waiting on a wait handle. + This should not cancel the wait if the APC has not been queued by runtime + as a result of internal interrupt handling. +*/ + +public class Test_wait_interrupted_user_apc +{ + public static bool Run115178Test => TestLibrary.Utilities.IsWindows; + + [DllImport("kernel32.dll")] + private static extern IntPtr GetCurrentProcess(); + + [DllImport("kernel32.dll")] + private static extern IntPtr GetCurrentThread(); + + [DllImport("kernel32.dll")] + private static extern uint QueueUserAPC( + IntPtr pfnAPC, + IntPtr hThread, + UIntPtr dwData); + + [DllImport("kernel32.dll")] + private static extern bool DuplicateHandle( + IntPtr hSourceProcessHandle, + IntPtr hSourceHandle, + IntPtr hTargetProcessHandle, + out IntPtr lpTargetHandle, + uint dwDesiredAccess, + bool bInheritHandle, + uint dwOptions); + + private delegate void ApcCallback(UIntPtr param); + + private static readonly ManualResetEventSlim waitEvent = new ManualResetEventSlim(false); + + private static readonly ManualResetEventSlim apcExecuted = new ManualResetEventSlim(false); + + private static IntPtr threadHandle; + + private static int result = 100; + + private static void OnApcCallback(UIntPtr param) + { + apcExecuted.Set(); + } + + private static void RunTestUsingInfiniteWait() + { + Console.WriteLine($"Running RunTestUsingInfiniteWait test."); + + var enterWait = new ManualResetEventSlim(false); + var leaveWait = new ManualResetEventSlim(false); + + apcExecuted.Reset(); + waitEvent.Reset(); + + new Thread(() => + { + Console.WriteLine($"Starting thread waiting on event."); + + IntPtr pseudoHandle = GetCurrentThread(); + if (!DuplicateHandle( + GetCurrentProcess(), + GetCurrentThread(), + GetCurrentProcess(), + out threadHandle, + 0, + false, + 2)) + { + Console.WriteLine($"Error duplicating handle, error code: {Marshal.GetLastWin32Error()}"); + result = 1; + } + + enterWait.Set(); + + bool signaled = waitEvent.Wait(Timeout.Infinite); + if (!signaled) + { + Console.WriteLine($"Error waiting on event, unknown user APC canceled wait."); + result = 2; + } + + signaled = waitEvent.Wait(0); + if (!signaled) + { + Console.WriteLine($"Error waiting on event, event should be signaled."); + result = 3; + } + + Console.WriteLine($"Stopping thread waiting on event."); + leaveWait.Set(); + }).Start(); + + ApcCallback callback = OnApcCallback; + IntPtr pfnAPC = Marshal.GetFunctionPointerForDelegate(callback); + + Console.WriteLine($"Waiting for thread to enter wait..."); + enterWait.Wait(Timeout.Infinite); + + Console.WriteLine($"Queue user APC."); + QueueUserAPC(pfnAPC, threadHandle, UIntPtr.Zero); + + Console.WriteLine($"Waiting for APC to execute..."); + apcExecuted.Wait(Timeout.Infinite); + + Console.WriteLine($"Signaling wait event."); + waitEvent.Set(); + + Console.WriteLine($"Waiting for thread to leave wait..."); + leaveWait.Wait(Timeout.Infinite); + + Console.WriteLine($"RunTestUsingInfiniteWait test executed."); + + GC.KeepAlive(callback); + } + + private static void RunTestUsingTimedWait() + { + Console.WriteLine($"Running RunTestUsingTimedWait test."); + + var enterWait = new ManualResetEventSlim(false); + var leaveWait = new ManualResetEventSlim(false); + + apcExecuted.Reset(); + waitEvent.Reset(); + + new Thread(() => + { + Console.WriteLine($"Starting thread waiting on event."); + + IntPtr pseudoHandle = GetCurrentThread(); + if (!DuplicateHandle( + GetCurrentProcess(), + GetCurrentThread(), + GetCurrentProcess(), + out threadHandle, + 0, + false, + 2)) + { + Console.WriteLine($"Error duplicating handle, error code: {Marshal.GetLastWin32Error()}"); + result = 4; + } + + enterWait.Set(); + + apcExecuted.Wait(Timeout.Infinite); + + var stopwatch = new Stopwatch(); + stopwatch.Start(); + + waitEvent.Wait(2000); + + stopwatch.Stop(); + + if (stopwatch.ElapsedMilliseconds < 2000) + { + Console.WriteLine($"Error waiting on event, wait returned too early."); + result = 5; + } + + Console.WriteLine($"Stopping thread waiting on event."); + leaveWait.Set(); + }).Start(); + + ApcCallback callback = OnApcCallback; + IntPtr pfnAPC = Marshal.GetFunctionPointerForDelegate(callback); + + Console.WriteLine($"Waiting for thread to enter wait..."); + enterWait.Wait(Timeout.Infinite); + + Console.WriteLine($"Queue user APC's."); + + do + { + QueueUserAPC(pfnAPC, threadHandle, UIntPtr.Zero); + } + while (!leaveWait.Wait(100)); + + Console.WriteLine($"Waiting for thread to leave wait..."); + leaveWait.Wait(Timeout.Infinite); + + Console.WriteLine($"RunTestUsingTimedWait test executed."); + + GC.KeepAlive(callback); + } + + private static void RunTestInterruptInfiniteWait() + { + Console.WriteLine($"Running RunTestInterruptInfiniteWait test."); + + var enterWait = new ManualResetEventSlim(false); + var leaveWait = new ManualResetEventSlim(false); + + apcExecuted.Reset(); + waitEvent.Reset(); + + var waitThread = new Thread(() => + { + Console.WriteLine($"Starting thread waiting on event."); + + IntPtr pseudoHandle = GetCurrentThread(); + if (!DuplicateHandle( + GetCurrentProcess(), + GetCurrentThread(), + GetCurrentProcess(), + out threadHandle, + 0, + false, + 2)) + { + Console.WriteLine($"Error duplicating handle, error code: {Marshal.GetLastWin32Error()}"); + result = 6; + } + + enterWait.Set(); + + try + { + var signaled = waitEvent.Wait(Timeout.Infinite); + if (!signaled) + { + Console.WriteLine($"Error waiting on event, unknown user APC canceled wait."); + result = 7; + } + } + catch (Exception ex) + { + if (ex is ThreadInterruptedException) + { + Console.WriteLine($"Thread was interrupted as expected."); + } + else + { + Console.WriteLine($"Unexpected exception: {ex.Message}"); + result = 8; + } + } + + Console.WriteLine($"Stopping thread waiting on event."); + leaveWait.Set(); + }); + + waitThread.Start(); + + ApcCallback callback = OnApcCallback; + IntPtr pfnAPC = Marshal.GetFunctionPointerForDelegate(callback); + + Console.WriteLine($"Waiting for thread to enter wait..."); + enterWait.Wait(Timeout.Infinite); + + Console.WriteLine($"Queue user APC."); + QueueUserAPC(pfnAPC, threadHandle, UIntPtr.Zero); + + Console.WriteLine($"Waiting for APC to execute..."); + apcExecuted.Wait(Timeout.Infinite); + + Console.WriteLine($"Interrupting thread wait..."); + waitThread.Interrupt(); + + Thread.Sleep(200); + + Console.WriteLine($"Signaling wait event."); + waitEvent.Set(); + + Console.WriteLine($"Waiting for thread to leave wait..."); + leaveWait.Wait(Timeout.Infinite); + + Console.WriteLine($"RunTestInterruptInfiniteWait test executed."); + + GC.KeepAlive(callback); + } + + [ConditionalFact(nameof(Run115178Test))] + public static int TestEntryPoint() + { + RunTestUsingInfiniteWait(); + RunTestUsingTimedWait(); + RunTestInterruptInfiniteWait(); + return result; + } +} diff --git a/src/tests/baseservices/threading/regressions/115178/115178.csproj b/src/tests/baseservices/threading/regressions/115178/115178.csproj new file mode 100644 index 00000000000000..58f25cbdd81c00 --- /dev/null +++ b/src/tests/baseservices/threading/regressions/115178/115178.csproj @@ -0,0 +1,9 @@ + + + true + + + + + +