Abort async suspend request on failure doing preemptive suspend. (#15486

)

When a preemptive suspend fails, state machine will get into an invalid
state and the thread is in an unknown state (running/self-suspended). In case of
failure in mono_threads_suspend_begin_async_suspend, a new method should be
called, mono_threads_transition_abort_async_suspend making sure state machine
reflects correct thread state as well as reporting correct result to initial
caller of mono_threads_suspend_begin_async_suspend.

mono/utils/mono-threads-mach.c

@@ -73,8 +73,16 @@ mono_threads_suspend_begin_async_suspend (MonoThreadInfo *info, gboolean interru
 	} while (ret == KERN_ABORTED);
 
 	THREADS_SUSPEND_DEBUG ("SUSPEND %p -> %d\n", (gpointer)(gsize)info->native_handle, ret);
-	if (ret != KERN_SUCCESS)
+	if (ret != KERN_SUCCESS) {
+		if (!mono_threads_transition_abort_async_suspend (info)) {
+			/* We raced with self suspend and lost so suspend can continue. */
+			g_assert (mono_threads_is_hybrid_suspension_enabled ());
+			info->suspend_can_continue = TRUE;
+			THREADS_SUSPEND_DEBUG ("\tlost race with self suspend %p\n", (gpointer)(gsize)info->native_handle);
+			return TRUE;
+		}
 		return FALSE;
+	}
 
 	if (!mono_threads_transition_finish_async_suspend (info)) {
 		/* We raced with self-suspend and lost.  Resume the native

mono/utils/mono-threads-posix.c

@@ -24,6 +24,7 @@
 #endif
 
 #include <mono/utils/mono-threads.h>
+#include <mono/utils/mono-threads-coop.h>
 #include <mono/utils/mono-coop-semaphore.h>
 #include <mono/metadata/gc-internals.h>
 #include <mono/utils/mono-threads-debug.h>
@@ -352,6 +353,13 @@ mono_threads_suspend_begin_async_suspend (MonoThreadInfo *info, gboolean interru
 		mono_threads_add_to_pending_operation_set (info);
 		return TRUE;
 	}
+	if (!mono_threads_transition_abort_async_suspend (info)) {
+		/* We raced with self suspend and lost so suspend can continue. */
+		g_assert (mono_threads_is_hybrid_suspension_enabled ());
+		info->suspend_can_continue = TRUE;
+		THREADS_SUSPEND_DEBUG ("\tlost race with self suspend %p\n", mono_thread_info_get_tid (info));
+		return TRUE;
+	}
 	return FALSE;
 }
 

mono/utils/mono-threads-state-machine.c

@@ -567,6 +567,59 @@ Don't expect these to be pulsed - they're not problematic.
 	}
 }
 
+/*
+Abort last step of preemptive suspend in case of failure to async suspend thread.
+This function makes sure state machine reflects current state of thread (running/suspended)
+in case of failure to complete async suspend of thread. NOTE, thread can still have reached
+a suspend state (in case of self-suspend).
+
+Returns TRUE if async suspend request was successfully aborted. Thread should be in STATE_RUNNING.
+Returns FALSE if async suspend request was successfully aborted but thread already reached self-suspended.
+*/
+gboolean
+mono_threads_transition_abort_async_suspend (MonoThreadInfo* info)
+{
+	int raw_state, cur_state, suspend_count;
+	gboolean no_safepoints;
+
+retry_state_change:
+	UNWRAP_THREAD_STATE (raw_state, cur_state, suspend_count, no_safepoints, info);
+	switch (cur_state) {
+	case STATE_SELF_SUSPENDED: //async suspend raced with self suspend and lost
+	case STATE_BLOCKING_SELF_SUSPENDED: //async suspend raced with blocking and lost
+		if (no_safepoints)
+			mono_fatal_with_history ("no_safepoints = TRUE, but should be FALSE");
+		trace_state_change_sigsafe ("ABORT_ASYNC_SUSPEND", info, raw_state, cur_state, no_safepoints, 0, "");
+		return FALSE; //thread successfully reached suspend state.
+	case STATE_ASYNC_SUSPEND_REQUESTED:
+	case STATE_BLOCKING_SUSPEND_REQUESTED:
+		if (!(suspend_count > 0))
+			mono_fatal_with_history ("suspend_count = %d, but should be > 0", suspend_count);
+		if (no_safepoints)
+			mono_fatal_with_history ("no_safepoints = TRUE, but should be FALSE");
+		if (suspend_count > 1) {
+			if (mono_atomic_cas_i32 (&info->thread_state, build_thread_state (cur_state, suspend_count - 1, no_safepoints), raw_state) != raw_state)
+				goto retry_state_change;
+			trace_state_change ("ABORT_ASYNC_SUSPEND", info, raw_state, cur_state, no_safepoints, -1);
+		} else {
+			if (mono_atomic_cas_i32 (&info->thread_state, build_thread_state (STATE_RUNNING, 0, no_safepoints), raw_state) != raw_state)
+				goto retry_state_change;
+			trace_state_change ("ABORT_ASYNC_SUSPEND", info, raw_state, STATE_RUNNING, no_safepoints, -1);
+		}
+		return TRUE; //aborting thread suspend request succeded, thread is running.
+
+/*
+STATE_RUNNING: A thread cannot escape suspension once requested.
+STATE_ASYNC_SUSPENDED: There can be only one suspend initiator at a given time, meaning this state should have been visible on the first stage of suspend.
+STATE_BLOCKING: If a thread is subject to preemptive suspend, there is no race as the resume initiator should have suspended the thread to STATE_BLOCKING_ASYNC_SUSPENDED or STATE_BLOCKING_SELF_SUSPENDED before resuming.
+				With cooperative suspend, there are no finish_async_suspend transitions since there's no path back from asyns_suspend requested to running.
+STATE_BLOCKING_ASYNC_SUSPENDED: There can only be one suspend initiator at a given time, meaning this state should have ben visible on the first stage of suspend.
+*/
+	default:
+		mono_fatal_with_history ("Cannot transition thread %p from %s with ABORT_ASYNC_SUSPEND", mono_thread_info_get_tid (info), state_name (cur_state));
+	}
+}
+
 /*
 This performs the last step of preemptive suspend.
 

mono/utils/mono-threads-windows.c

@@ -182,6 +182,13 @@ mono_threads_suspend_begin_async_suspend (MonoThreadInfo *info, gboolean interru
 	result = SuspendThread (handle);
 	THREADS_SUSPEND_DEBUG ("SUSPEND %p -> %u\n", GUINT_TO_POINTER (id), result);
 	if (result == (DWORD)-1) {
+		if (!mono_threads_transition_abort_async_suspend (info)) {
+			/* We raced with self suspend and lost so suspend can continue. */
+			g_assert (mono_threads_is_hybrid_suspension_enabled ());
+			info->suspend_can_continue = TRUE;
+			THREADS_SUSPEND_DEBUG ("\tlost race with self suspend %p\n", mono_thread_info_get_tid (info));
+			return TRUE;
+		}
 		THREADS_SUSPEND_DEBUG ("SUSPEND FAILED, id=%p, err=%u\n", GUINT_TO_POINTER (id), GetLastError ());
 		return FALSE;
 	}
@@ -195,9 +202,16 @@ mono_threads_suspend_begin_async_suspend (MonoThreadInfo *info, gboolean interru
 	CONTEXT context;
 	context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
 	if (!GetThreadContext (handle, &context)) {
-		THREADS_SUSPEND_DEBUG ("SUSPEND FAILED (GetThreadContext), id=%p, err=%u\n", GUINT_TO_POINTER (id), GetLastError ());
 		result = ResumeThread (handle);
 		g_assert (result == 1);
+		if (!mono_threads_transition_abort_async_suspend (info)) {
+			/* We raced with self suspend and lost so suspend can continue. */
+			g_assert (mono_threads_is_hybrid_suspension_enabled ());
+			info->suspend_can_continue = TRUE;
+			THREADS_SUSPEND_DEBUG ("\tlost race with self suspend %p\n", mono_thread_info_get_tid (info));
+			return TRUE;
+		}
+		THREADS_SUSPEND_DEBUG ("SUSPEND FAILED (GetThreadContext), id=%p, err=%u\n", GUINT_TO_POINTER (id), GetLastError ());
 		return FALSE;
 	}
 

mono/utils/mono-threads.h

@@ -563,7 +563,10 @@ This begins async suspend. This function must do the following:
 -Call mono_threads_transition_finish_async_suspend as part of its async suspend.
 -Register the thread for pending suspend with mono_threads_add_to_pending_operation_set if needed.
 
-If begin suspend fails the thread must be left uninterrupted and resumed.
+If begin suspend fails the following should be done:
+-The thread must be left uninterrupted and resumed.
+-Call mono_threads_transition_abort_async_suspend to make sure thread has correct state.
+-No pending suspend should be registered with mono_threads_add_to_pending_operation_set for this operation.
 */
 gboolean mono_threads_suspend_begin_async_suspend (THREAD_INFO_TYPE *info, gboolean interrupt_kernel);
 
@@ -707,6 +710,7 @@ MonoRequestSuspendResult mono_threads_transition_request_suspension (THREAD_INFO
 MonoSelfSupendResult mono_threads_transition_state_poll (THREAD_INFO_TYPE *info);
 MonoResumeResult mono_threads_transition_request_resume (THREAD_INFO_TYPE* info);
 MonoPulseResult mono_threads_transition_request_pulse (THREAD_INFO_TYPE* info);
+gboolean mono_threads_transition_abort_async_suspend (THREAD_INFO_TYPE* info);
 gboolean mono_threads_transition_finish_async_suspend (THREAD_INFO_TYPE* info);
 MonoDoBlockingResult mono_threads_transition_do_blocking (THREAD_INFO_TYPE* info, const char* func);
 MonoDoneBlockingResult mono_threads_transition_done_blocking (THREAD_INFO_TYPE* info, const char* func);