Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 28, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f1859b
Only interrupt wait on Windows when User APC's are queued by Mono.
lateralusX May 23, 2025
915f16d
Adding test.
lateralusX May 26, 2025
bd133ed
Fix mono_monitor_wait to raise thread iterrupted exception.
lateralusX May 27, 2025
e307e44
Add interrupt wait test + enable test on CoreCLR.
lateralusX May 27, 2025
464b5a6
Test runs on both runtimes.
lateralusX May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/mono/mono/metadata/monitor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
110 changes: 85 additions & 25 deletions src/mono/mono/utils/mono-os-wait-win32.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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)

Expand All @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down
24 changes: 18 additions & 6 deletions src/mono/mono/utils/mono-threads-windows.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/mono/mono/utils/mono-threads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading