Skip to content

Commit f1d0dfe

Browse files
authored
[mono][threads] save/restore errno in thread state transitions (#79856)
* [threads] Save errno when using posix semaphores for thread transitions We already save/restore GetLastError on win32. Do it on posix platforms, too. If one thread is in a pinvoke wrapper, while another thread triggers a STW, the pinvoke wrapper will self-suspend the thread and wait for a notification to resume. Depending on the platform we can use win32 primitives, Mach semaphores or POSIX semaphores. win32 and posix can both change the value of last error (errno, respectively) while the thread is suspended. That means that code like this (generated by the LibraryImportAttribute source generator) cannot reliably retrieve the error from the last pinvoke: ```csharp __retVal = __PInvoke(__path_native, mode); // there is a pinvoke wrapper here, that transitions from GC Safe to GC Unsafe mode __lastError = System.Runtime.InteropServices.Marshal.GetLastSystemError(); ``` The solution is to explicitly preserve the value of GetLastError/errno when exiting from GC Safe. Fixes #77364 * rename W32_DEFINE_LAST_ERROR_RESTORE_POINT to MONO_DEFINE_LAST_ERROR_RESTORE_POINT and W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT to MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT
1 parent 693eef6 commit f1d0dfe

File tree

3 files changed

+44
-13
lines changed

3 files changed

+44
-13
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -339,10 +339,10 @@ mono_threads_exit_gc_safe_region_internal (gpointer cookie, MonoStackData *stack
339339
return;
340340

341341
#ifdef ENABLE_CHECKED_BUILD_GC
342-
W32_DEFINE_LAST_ERROR_RESTORE_POINT;
342+
MONO_DEFINE_LAST_ERROR_RESTORE_POINT;
343343
if (mono_check_mode_enabled (MONO_CHECK_MODE_GC))
344344
coop_tls_pop (cookie);
345-
W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
345+
MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
346346
#endif
347347

348348
mono_threads_exit_gc_safe_region_unbalanced_internal (cookie, stackdata);
@@ -367,7 +367,7 @@ mono_threads_exit_gc_safe_region_unbalanced_internal (gpointer cookie, MonoStack
367367
/* Common to use enter/exit gc safe around OS API's affecting last error. */
368368
/* This method can call OS API's that will reset last error on some platforms. */
369369
/* To reduce errors, we need to restore last error before exit gc safe. */
370-
W32_DEFINE_LAST_ERROR_RESTORE_POINT;
370+
MONO_DEFINE_LAST_ERROR_RESTORE_POINT;
371371

372372
info = (MonoThreadInfo *)cookie;
373373

@@ -400,7 +400,7 @@ mono_threads_exit_gc_safe_region_unbalanced_internal (gpointer cookie, MonoStack
400400
info->user_data = NULL;
401401
}
402402

403-
W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
403+
MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
404404
}
405405

406406
void
@@ -654,14 +654,14 @@ mono_threads_suspend_policy_init (void)
654654
// otherwise if one of the old environment variables is set, use that.
655655
// otherwise use full preemptive suspend.
656656

657-
W32_DEFINE_LAST_ERROR_RESTORE_POINT;
657+
MONO_DEFINE_LAST_ERROR_RESTORE_POINT;
658658

659659
(policy = threads_suspend_policy_getenv ())
660660
|| (policy = threads_suspend_policy_default ())
661661
|| (policy = threads_suspend_policy_getenv_compat ())
662662
|| (policy = MONO_THREADS_SUSPEND_FULL_PREEMPTIVE);
663663

664-
W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
664+
MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
665665

666666
g_assert (policy);
667667
mono_threads_suspend_policy_hidden_dont_modify = (char)policy;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,7 +1915,7 @@ mono_thread_info_uninstall_interrupt (gboolean *interrupted)
19151915
/* Common to uninstall interrupt handler around OS API's affecting last error. */
19161916
/* This method could call OS API's on some platforms that will reset last error so make sure to restore */
19171917
/* last error before exit. */
1918-
W32_DEFINE_LAST_ERROR_RESTORE_POINT;
1918+
MONO_DEFINE_LAST_ERROR_RESTORE_POINT;
19191919

19201920
g_assert (interrupted);
19211921
*interrupted = FALSE;
@@ -1938,7 +1938,7 @@ mono_thread_info_uninstall_interrupt (gboolean *interrupted)
19381938
THREADS_INTERRUPT_DEBUG ("interrupt uninstall tid %p previous_token %p interrupted %s\n",
19391939
mono_thread_info_get_tid (info), previous_token, *interrupted ? "TRUE" : "FALSE");
19401940

1941-
W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
1941+
MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT;
19421942
}
19431943

19441944
static MonoThreadInfoInterruptToken*

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

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -871,19 +871,50 @@ mono_win32_interrupt_wait (PVOID thread_info, HANDLE native_thread_handle, DWORD
871871
void
872872
mono_win32_abort_blocking_io_call (THREAD_INFO_TYPE *info);
873873

874-
#define W32_DEFINE_LAST_ERROR_RESTORE_POINT \
874+
#else
875+
876+
877+
#endif
878+
879+
#ifdef USE_WINDOWS_BACKEND
880+
881+
/* APC calls can change GetLastError while a thread is suspended. Save/restore it when doing thread
882+
state transitions (for example in m2n wrappers) in order to protect the result of the last
883+
pinvoke */
884+
885+
#define MONO_DEFINE_LAST_ERROR_RESTORE_POINT \
875886
const DWORD _last_error_restore_point = GetLastError ();
876887

877-
#define W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT \
888+
#define MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT \
878889
/* Only restore if changed to prevent unnecessary writes. */ \
879890
if (GetLastError () != _last_error_restore_point) \
880891
mono_SetLastError (_last_error_restore_point);
881892

882-
#else
893+
#elif defined(USE_WASM_BACKEND) || defined (USE_POSIX_BACKEND)
883894

884-
#define W32_DEFINE_LAST_ERROR_RESTORE_POINT /* nothing */
885-
#define W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT /* nothing */
895+
#define MONO_DEFINE_LAST_ERROR_RESTORE_POINT \
896+
int _last_errno_restore_point = errno;
886897

898+
#define MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT \
899+
if (errno != _last_errno_restore_point) \
900+
errno = _last_errno_restore_point;
901+
902+
/* Posix semaphores set errno on failure and sporadic wakeup. GC state transitions are done in n2m
903+
* and m2n wrappers and may change the value of errno from the last pinvoke. Use these macros to
904+
* save/restore errno when doing thread state transitions. */
905+
906+
#elif defined(USE_MACH_BACKEND)
907+
908+
/* Mach semaphores don't set errno on failure. Change this to be the same as POSIX if some other primitives used
909+
in thread state transitions pollute errno. */
910+
911+
#define MONO_DEFINE_LAST_ERROR_RESTORE_POINT /* nothing */
912+
#define MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT /* nothing */
913+
914+
#else
915+
#error "unknown threads backend, not sure how to save/restore last error"
887916
#endif
888917

918+
919+
889920
#endif /* __MONO_THREADS_H__ */

0 commit comments

Comments
 (0)