[Mono]: Additional support for signal async safe stack-walks. (#113645)

lateralusX · web-flow · commit d34ef7e2d3f4 · 2025-03-20T11:20:08.000+01:00
* [Mono]: Fix additional stack-walks to be async safe. Mono's stack-walker can be signal/async safe when needed, making sure it won't allocate additional memory or taking internal runtime locks. It is however up to the caller of the stack walking API's to decide if it should be signal and/or async safe. Currently this is controlled using two different flags, the MONO_UNWIND_SIGNAL_SAFE as well as mono_thread_info_is_async_context. This is problematic since callers wants signal safe stack-walking but since not both are set, it will not behave fully signal safe. dotnet/android#9365 hit a couple of scenarios described here: dotnet/android#9365 (comment) that ends up deadlocking due to the fact that they did stack-walking from within a signal handler and deadlocked or dumping stack on suspended thread holding runtime loader lock, but without making the stack-walk async safe. Fix makes sure that calls to stack-walk API's can be made signal and/or async safe and that identified areas uses the correct set of flags given state of threads when stack-walking. * Add signal async safe stack unwind option. * Assert that walk_stack_full_llvm_only is only called in llvm only mode. * Correct some bool usage. * Make signal safe unwind option, signal async safe. Mono's current MONO_UNWIND_SIGNAL_SAFE was not fully signal safe since it was not async safe, that could lead to taking loader lock. This will fix MONO_UNWIND_SIGNAL_SAFE to be signal asycn safe, it will also change current use of MONO_UNWIND_SIGNAL_SAFE to MONO_UNWIND_NONE since they where equal before this fix, meaning old calls using MONO_UNWIND_SIGNAL_SAFE will behave identical using MONO_UNWIND_NONE so no regression.
diff --git a/src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c b/src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c
@@ -1184,31 +1184,27 @@ ep_rt_mono_walk_managed_stack_for_thread (
 	stack_walk_data.safe_point_frame = false;
 	stack_walk_data.runtime_invoke_frame = false;
 
-	bool restore_async_context = FALSE;
-	bool prevent_profiler_event_recursion = FALSE;
+	bool prevent_profiler_event_recursion = false;
+	MonoUnwindOptions unwind_options = MONO_UNWIND_NONE;
 	EventPipeMonoThreadData *thread_data = ep_rt_mono_thread_data_get_or_create ();
 	if (thread_data) {
 		prevent_profiler_event_recursion = thread_data->prevent_profiler_event_recursion;
-		if (prevent_profiler_event_recursion && !mono_thread_info_is_async_context ()) {
+		if (prevent_profiler_event_recursion) {
 			// Running stackwalk in async context mode is currently the only way to prevent
 			// unwinder to NOT load additional classes during stackwalk, making it signal unsafe and
 			// potential triggering uncontrolled recursion in profiler class loading event.
-			mono_thread_info_set_is_async_context (TRUE);
-			restore_async_context = TRUE;
+			unwind_options = MONO_UNWIND_SIGNAL_SAFE;
 		}
-		thread_data->prevent_profiler_event_recursion = TRUE;
+		thread_data->prevent_profiler_event_recursion = true;
 	}
 
 	if (thread == ep_rt_thread_get_handle () && mono_get_eh_callbacks ()->mono_walk_stack_with_ctx)
-		mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (walk_managed_stack_for_thread_callback, NULL, MONO_UNWIND_SIGNAL_SAFE, &stack_walk_data);
+		mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (walk_managed_stack_for_thread_callback, NULL, unwind_options, &stack_walk_data);
 	else if (mono_get_eh_callbacks ()->mono_walk_stack_with_state)
-		mono_get_eh_callbacks ()->mono_walk_stack_with_state (walk_managed_stack_for_thread_callback, mono_thread_info_get_suspend_state (thread), MONO_UNWIND_SIGNAL_SAFE, &stack_walk_data);
+		mono_get_eh_callbacks ()->mono_walk_stack_with_state (walk_managed_stack_for_thread_callback, mono_thread_info_get_suspend_state (thread), unwind_options, &stack_walk_data);
 
-	if (thread_data) {
-		if (restore_async_context)
-			mono_thread_info_set_is_async_context (FALSE);
+	if (thread_data)
 		thread_data->prevent_profiler_event_recursion = prevent_profiler_event_recursion;
-	}
 
 	return true;
 }
@@ -1236,10 +1232,10 @@ ep_rt_mono_sample_profiler_write_sampling_event_for_threads (
 
 	mono_stop_world (MONO_THREAD_INFO_FLAGS_NO_GC);
 
-	bool restore_async_context = FALSE;
+	bool restore_async_context = false;
 	if (!mono_thread_info_is_async_context ()) {
 		mono_thread_info_set_is_async_context (TRUE);
-		restore_async_context = TRUE;
+		restore_async_context = true;
 	}
 
 	// Record all info needed in sample events while runtime is suspended, must be async safe.
@@ -1361,7 +1357,7 @@ method_enter (MonoProfiler *prof, MonoMethod *method, MonoProfilerCallContext *c
 	data->stack_walk_data.runtime_invoke_frame = false;
 	ep_stack_contents_reset (&data->stack_contents);
 
-	mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (sample_profiler_walk_managed_stack_for_thread_callback, &ctx->context, MONO_UNWIND_SIGNAL_SAFE, &stack_walk_data);
+	mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (sample_profiler_walk_managed_stack_for_thread_callback, &ctx->context, MONO_UNWIND_NONE, &stack_walk_data);
 	if (data->payload_data == EP_SAMPLE_PROFILER_SAMPLE_TYPE_EXTERNAL && (data->stack_walk_data.safe_point_frame || data->stack_walk_data.runtime_invoke_frame)) {
 		data->payload_data = EP_SAMPLE_PROFILER_SAMPLE_TYPE_MANAGED;
 	}
@@ -2504,7 +2500,7 @@ write_event_exception_thrown (MonoObject *obj)
 			exception_message = g_strdup ("");
 
 		if (mono_get_eh_callbacks ()->mono_walk_stack_with_ctx)
-			mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (get_exception_ip_func, NULL, MONO_UNWIND_SIGNAL_SAFE, (void *)&ip);
+			mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (get_exception_ip_func, NULL, MONO_UNWIND_NONE, (void *)&ip);
 
 		type_name = mono_type_get_name_full (m_class_get_byval_arg (mono_object_class (obj)), MONO_TYPE_NAME_FORMAT_IL);
 
@@ -3024,13 +3020,13 @@ class_loading_callback (
 	MonoProfiler *prof,
 	MonoClass *klass)
 {
-	bool prevent_profiler_event_recursion = FALSE;
+	bool prevent_profiler_event_recursion = false;
 	EventPipeMonoThreadData *thread_data = ep_rt_mono_thread_data_get_or_create ();
 	if (thread_data) {
 		// Prevent additional class loading to happen recursively as part of fire TypeLoadStart event.
 		// Additional class loading can happen as part of capturing callstack for TypeLoadStart event.
 		prevent_profiler_event_recursion = thread_data->prevent_profiler_event_recursion;
-		thread_data->prevent_profiler_event_recursion = TRUE;
+		thread_data->prevent_profiler_event_recursion = true;
 	}
 
 	write_event_type_load_start (m_class_get_byval_arg (klass));
diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c
@@ -2941,20 +2941,13 @@ collect_frame (MonoStackFrameInfo *frame, MonoContext *ctx, gpointer data)
 	return FALSE;
 }
 
-/* This needs to be async safe */
 static SuspendThreadResult
 get_thread_dump (MonoThreadInfo *info, gpointer ud)
 {
 	ThreadDumpUserData *user_data = (ThreadDumpUserData *)ud;
 	MonoInternalThread *thread = user_data->thread;
 
-#if 0
-/* This no longer works with remote unwinding */
-	g_string_append_printf (text, " tid=0x%p this=0x%p ", (gpointer)(gsize)thread->tid, thread);
-	mono_thread_internal_describe (thread, text);
-	g_string_append (text, "\n");
-#endif
-
+	/* This needs to be async safe */
 	if (thread == mono_thread_internal_current ())
 		mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (collect_frame, NULL, MONO_UNWIND_SIGNAL_SAFE, ud);
 	else
@@ -4107,9 +4100,8 @@ mono_thread_info_get_last_managed (MonoThreadInfo *info)
 	 * The suspended thread might be holding runtime locks. Make sure we don't try taking
 	 * any runtime locks while unwinding.
 	 */
-	mono_thread_info_set_is_async_context (TRUE);
 	mono_get_eh_callbacks ()->mono_walk_stack_with_state (last_managed, mono_thread_info_get_suspend_state (info), MONO_UNWIND_SIGNAL_SAFE, &ji);
-	mono_thread_info_set_is_async_context (FALSE);
+
 	return ji;
 }
 
diff --git a/src/mono/mono/mini/mini-exceptions.c b/src/mono/mono/mini/mini-exceptions.c
@@ -126,7 +126,7 @@ static MonoFtnPtrEHCallback ftnptr_eh_callback;
  */
 int mono_llvmonly_do_unwind_flag;
 
-static void mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx, MonoJitTlsData *jit_tls, MonoLMF *lmf, MonoUnwindOptions unwind_options, gpointer user_data, gboolean crash_context);
+static void mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx, MonoJitTlsData *jit_tls, MonoLMF *lmf, MonoUnwindOptions unwind_options, gpointer user_data);
 static void mono_raise_exception_with_ctx (MonoException *exc, MonoContext *ctx);
 static void mono_runtime_walk_stack_with_ctx (MonoJitStackWalk func, MonoContext *start_ctx, MonoUnwindOptions unwind_options, void *user_data);
 static gboolean mono_current_thread_has_handle_block_guard (void);
@@ -160,7 +160,7 @@ static gpointer
 mono_thread_get_managed_sp (void)
 {
 	gpointer addr = NULL;
-	mono_walk_stack (first_managed, MONO_UNWIND_SIGNAL_SAFE, &addr);
+	mono_walk_stack (first_managed, MONO_UNWIND_NONE, &addr);
 	return addr;
 }
 
@@ -1224,7 +1224,7 @@ mono_walk_stack_with_ctx (MonoJitStackWalk func, MonoContext *start_ctx, MonoUnw
 		start_ctx = &extra_ctx;
 	}
 
-	mono_walk_stack_full (func, start_ctx, thread->jit_data, mono_get_lmf (), unwind_options, user_data, FALSE);
+	mono_walk_stack_full (func, start_ctx, thread->jit_data, mono_get_lmf (), unwind_options, user_data);
 }
 
 /**
@@ -1241,8 +1241,9 @@ void
 mono_walk_stack_with_state (MonoJitStackWalk func, MonoThreadUnwindState *state, MonoUnwindOptions unwind_options, void *user_data)
 {
 	MonoThreadUnwindState extra_state;
+
 	if (!state) {
-		g_assert (!mono_thread_info_is_async_context ());
+		g_assert (!(unwind_options & MONO_UNWIND_SIGNAL_SAFE) && !mono_thread_info_is_async_context ());
 		if (!mono_thread_state_init_from_current (&extra_state))
 			return;
 		state = &extra_state;
@@ -1258,7 +1259,7 @@ mono_walk_stack_with_state (MonoJitStackWalk func, MonoThreadUnwindState *state,
 		&state->ctx,
 		(MonoJitTlsData *)state->unwind_data [MONO_UNWIND_DATA_JIT_TLS],
 		(MonoLMF *)state->unwind_data [MONO_UNWIND_DATA_LMF],
-		unwind_options, user_data, FALSE);
+		unwind_options, user_data);
 }
 
 void
@@ -1270,22 +1271,49 @@ mono_walk_stack (MonoJitStackWalk func, MonoUnwindOptions options, void *user_da
 	mono_walk_stack_with_state (func, &state, options, user_data);
 }
 
-/**
- * mono_walk_stack_full:
- * \param func callback to call for each stack frame
- * \param unwind_options what extra information the unwinder should gather
- * \param start_ctx starting state of the stack walk, can be NULL.
- * \param thread the thread whose stack to walk, can be NULL to use the current thread
- * \param lmf the LMF of \p thread, can be NULL to use the LMF of the current thread
- * \param user_data data passed to the callback
- * \param crash_context tells us that we're in a context where it's not safe to lock or allocate
- * This function walks the stack of a thread, starting from the state
- * represented by \p start_ctx. For each frame the callback
- * function is called with the relevant info. The walk ends when no more
- * managed stack frames are found or when the callback returns a TRUE value.
- */
+#ifndef TARGET_WASM
 static void
-mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx, MonoJitTlsData *jit_tls, MonoLMF *lmf, MonoUnwindOptions unwind_options, gpointer user_data, gboolean crash_context)
+walk_stack_full_llvm_only(MonoJitStackWalk func, MonoContext* start_ctx, MonoJitTlsData* jit_tls, MonoLMF* lmf, MonoUnwindOptions unwind_options, gpointer user_data)
+{
+	GSList *l, *ips;
+	StackFrameInfo frame;
+
+	g_assert(mono_llvm_only);
+
+	memset (&frame, 0, sizeof (StackFrameInfo));
+
+	// TODO: Fix async support.
+	if (mono_thread_info_is_async_context ())
+		return;
+
+	ips = get_unwind_backtrace ();
+	for (l = ips; l; l = l->next) {
+		guint8 *ip = (guint8*)l->data;
+		memset (&frame, 0, sizeof (StackFrameInfo));
+		frame.ji = mini_jit_info_table_find (ip);
+		if (!frame.ji || frame.ji->is_trampoline)
+			continue;
+		frame.type = FRAME_TYPE_MANAGED;
+		frame.method = jinfo_get_method (frame.ji);
+		// FIXME: Cannot lookup the actual method
+		frame.actual_method = frame.method;
+		if (frame.type == FRAME_TYPE_MANAGED) {
+			if (!frame.method->wrapper_type || frame.method->wrapper_type == MONO_WRAPPER_DYNAMIC_METHOD)
+				frame.managed = TRUE;
+		}
+		frame.native_offset = GPTRDIFF_TO_INT (ip - (guint8*)frame.ji->code_start);
+		frame.il_offset = -1;
+
+		if (func (&frame, NULL, user_data))
+			break;
+	}
+	g_slist_free (ips);
+	return;
+}
+#endif
+
+static void
+walk_stack_full(MonoJitStackWalk func, MonoContext* start_ctx, MonoJitTlsData* jit_tls, MonoLMF* lmf, MonoUnwindOptions unwind_options, gpointer user_data)
 {
 	gint il_offset;
 	MonoContext ctx, new_ctx;
@@ -1299,39 +1327,6 @@ mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx, MonoJitTlsD
 
 	memset (&frame, 0, sizeof (StackFrameInfo));
 
-#ifndef TARGET_WASM
-	if (mono_llvm_only) {
-		GSList *l, *ips;
-
-		if (async)
-			return;
-
-		ips = get_unwind_backtrace ();
-		for (l = ips; l; l = l->next) {
-			guint8 *ip = (guint8*)l->data;
-			memset (&frame, 0, sizeof (StackFrameInfo));
-			frame.ji = mini_jit_info_table_find (ip);
-			if (!frame.ji || frame.ji->is_trampoline)
-				continue;
-			frame.type = FRAME_TYPE_MANAGED;
-			frame.method = jinfo_get_method (frame.ji);
-			// FIXME: Cannot lookup the actual method
-			frame.actual_method = frame.method;
-			if (frame.type == FRAME_TYPE_MANAGED) {
-				if (!frame.method->wrapper_type || frame.method->wrapper_type == MONO_WRAPPER_DYNAMIC_METHOD)
-					frame.managed = TRUE;
-			}
-			frame.native_offset = GPTRDIFF_TO_INT (ip - (guint8*)frame.ji->code_start);
-			frame.il_offset = -1;
-
-			if (func (&frame, NULL, user_data))
-				break;
-		}
-		g_slist_free (ips);
-		return;
-	}
-#endif
-
 	if (!start_ctx) {
 		g_warning ("start_ctx required for stack walk");
 		return;
@@ -1372,15 +1367,15 @@ mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx, MonoJitTlsD
 			MonoDebugSourceLocation *source = NULL;
 
 			// Don't do this when we can be in a signal handler
-			if (!crash_context)
+			if (!async)
 				source = mono_debug_lookup_source_location (jinfo_get_method (frame.ji), frame.native_offset, NULL);
 			if (source) {
 				il_offset = source->il_offset;
 			} else {
 				MonoSeqPointInfo *seq_points = NULL;
 
 				// It's more reliable to look into the global cache if possible
-				if (crash_context)
+				if (async)
 					seq_points = (MonoSeqPointInfo *) frame.ji->seq_points;
 				else
 					seq_points = mono_get_seq_points (jinfo_get_method (frame.ji));
@@ -1422,6 +1417,41 @@ mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx, MonoJitTlsD
 	}
 }
 
+/**
+ * mono_walk_stack_full:
+ * \param func callback to call for each stack frame
+ * \param unwind_options what extra information the unwinder should gather
+ * \param start_ctx starting state of the stack walk, can be NULL.
+ * \param thread the thread whose stack to walk, can be NULL to use the current thread
+ * \param lmf the LMF of \p thread, can be NULL to use the LMF of the current thread
+ * \param user_data data passed to the callback
+ * This function walks the stack of a thread, starting from the state
+ * represented by \p start_ctx. For each frame the callback
+ * function is called with the relevant info. The walk ends when no more
+ * managed stack frames are found or when the callback returns a TRUE value.
+ */
+static void
+mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx, MonoJitTlsData *jit_tls, MonoLMF *lmf, MonoUnwindOptions unwind_options, gpointer user_data)
+{
+	gboolean restore_async_context = FALSE;
+	if ((unwind_options & MONO_UNWIND_SIGNAL_SAFE) && !mono_thread_info_is_async_context ()) {
+		mono_thread_info_set_is_async_context (TRUE);
+		restore_async_context = TRUE;
+	}
+
+#ifndef TARGET_WASM
+	if (mono_llvm_only)
+		walk_stack_full_llvm_only (func, start_ctx, jit_tls, lmf, unwind_options, user_data);
+	else
+		walk_stack_full (func, start_ctx, jit_tls, lmf, unwind_options, user_data);
+#else
+	walk_stack_full (func, start_ctx, jit_tls, lmf, unwind_options, user_data);
+#endif
+
+	if (restore_async_context)
+		mono_thread_info_set_is_async_context (FALSE);
+}
+
 MonoBoolean
 mono_get_frame_info (gint32 skip,
 					 MonoMethod **out_method,
@@ -2976,7 +3006,8 @@ mono_handle_native_crash (const char *signal, MonoContext *mctx, MONO_SIG_HANDLE
 		g_async_safe_printf ("\tManaged Stacktrace:\n");
 		g_async_safe_printf ("=================================================================\n");
 
-		mono_walk_stack_full (print_stack_frame_signal_safe, mctx, jit_tls, mono_get_lmf (), MONO_UNWIND_LOOKUP_IL_OFFSET, NULL, TRUE);
+		mono_walk_stack_full (print_stack_frame_signal_safe, mctx, jit_tls, mono_get_lmf (), MONO_UNWIND_LOOKUP_IL_OFFSET | MONO_UNWIND_SIGNAL_SAFE, NULL);
+
 		g_async_safe_printf ("=================================================================\n");
 	}
 
@@ -3146,15 +3177,13 @@ mono_install_handler_block_guard (MonoThreadUnwindState *ctx)
 	MonoJitTlsData *jit_tls = (MonoJitTlsData *)ctx->unwind_data [MONO_UNWIND_DATA_JIT_TLS];
 
 	/* Guard against a null MonoJitTlsData. This can happens if the thread receives the
-         * interrupt signal before the JIT has time to initialize its TLS data for the given thread.
+	 * interrupt signal before the JIT has time to initialize its TLS data for the given thread.
 	 */
 	if (!jit_tls || jit_tls->handler_block)
 		return FALSE;
 
 	/* Do an async safe stack walk */
-	mono_thread_info_set_is_async_context (TRUE);
-	mono_walk_stack_with_state (find_last_handler_block, ctx, MONO_UNWIND_NONE, &data);
-	mono_thread_info_set_is_async_context (FALSE);
+	mono_walk_stack_with_state (find_last_handler_block, ctx, MONO_UNWIND_SIGNAL_SAFE, &data);
 
 	if (!data.ji)
 		return FALSE;
diff --git a/src/mono/mono/mini/mini-posix.c b/src/mono/mono/mini/mini-posix.c
@@ -291,11 +291,16 @@ MONO_SIG_HANDLER_FUNC (static, profiler_signal_handler)
 
 	int hp_save_index = mono_hazard_pointer_save_for_signal_handler ();
 
-	mono_thread_info_set_is_async_context (TRUE);
+	gboolean restore_async_context = FALSE;
+	if (!mono_thread_info_is_async_context ()) {
+		mono_thread_info_set_is_async_context (TRUE);
+		restore_async_context = TRUE;
+	}
 
 	MONO_PROFILER_RAISE (sample_hit, ((const mono_byte*)mono_arch_ip_from_context (ctx), ctx));
 
-	mono_thread_info_set_is_async_context (FALSE);
+	if (restore_async_context)
+		mono_thread_info_set_is_async_context (FALSE);
 
 	mono_hazard_pointer_restore_for_signal_handler (hp_save_index);
 
diff --git a/src/mono/mono/utils/mono-stack-unwinding.h b/src/mono/mono/utils/mono-stack-unwinding.h
@@ -48,8 +48,8 @@ typedef enum {
 	 * callback.
 	 */
 	MONO_UNWIND_REG_LOCATIONS = 0x4,
+	MONO_UNWIND_SIGNAL_SAFE = 0x8,
 	MONO_UNWIND_DEFAULT = MONO_UNWIND_LOOKUP_ACTUAL_METHOD,
-	MONO_UNWIND_SIGNAL_SAFE = MONO_UNWIND_NONE,
 	MONO_UNWIND_LOOKUP_ALL = MONO_UNWIND_LOOKUP_IL_OFFSET | MONO_UNWIND_LOOKUP_ACTUAL_METHOD,
 } MonoUnwindOptions;
 
