From 1e4f6b185b2aab27d5d004626874215c30720651 Mon Sep 17 00:00:00 2001 From: jaykrell Date: Tue, 16 Jan 2018 12:55:34 -0800 Subject: [PATCH] Convert thread get/set/clear state to coop/handle. (#6389) * Coop-handle ves_icall_System_Threading_Thread_GetState, SetState, ClrState. And cleanup. * Favor mono_internal_thread_handle_ptr. --- mono/metadata/icall-def.h | 6 +-- mono/metadata/object-internals.h | 6 --- mono/metadata/threadpool.c | 6 +-- mono/metadata/threads-types.h | 13 +++++-- mono/metadata/threads.c | 80 +++++++++++++++++++++++----------------- 5 files changed, 63 insertions(+), 48 deletions(-) diff --git a/mono/metadata/icall-def.h b/mono/metadata/icall-def.h index 4a231fdd9d3..c95ab3c601d 100644 --- a/mono/metadata/icall-def.h +++ b/mono/metadata/icall-def.h @@ -934,7 +934,7 @@ ICALL_TYPE(THREAD, "System.Threading.Thread", THREAD_1) ICALL(THREAD_1, "Abort_internal(System.Threading.InternalThread,object)", ves_icall_System_Threading_Thread_Abort) ICALL(THREAD_1a, "ByteArrayToCurrentDomain(byte[])", ves_icall_System_Threading_Thread_ByteArrayToCurrentDomain) ICALL(THREAD_1b, "ByteArrayToRootDomain(byte[])", ves_icall_System_Threading_Thread_ByteArrayToRootDomain) -ICALL(THREAD_2, "ClrState(System.Threading.InternalThread,System.Threading.ThreadState)", ves_icall_System_Threading_Thread_ClrState) +HANDLES(ICALL(THREAD_2, "ClrState(System.Threading.InternalThread,System.Threading.ThreadState)", ves_icall_System_Threading_Thread_ClrState)) ICALL(THREAD_2a, "ConstructInternalThread", ves_icall_System_Threading_Thread_ConstructInternalThread) ICALL(THREAD_55, "GetAbortExceptionState", ves_icall_System_Threading_Thread_GetAbortExceptionState) HANDLES(ICALL(THREAD_60, "GetCurrentThread", ves_icall_System_Threading_Thread_GetCurrentThread)) @@ -942,7 +942,7 @@ ICALL(THREAD_7, "GetDomainID", ves_icall_System_Threading_Thread_GetDomainID) ICALL(THREAD_8, "GetName_internal(System.Threading.InternalThread)", ves_icall_System_Threading_Thread_GetName_internal) HANDLES(ICALL(THREAD_57, "GetPriorityNative", ves_icall_System_Threading_Thread_GetPriority)) ICALL(THREAD_59, "GetStackTraces", ves_icall_System_Threading_Thread_GetStackTraces) -ICALL(THREAD_11, "GetState(System.Threading.InternalThread)", ves_icall_System_Threading_Thread_GetState) +HANDLES(ICALL(THREAD_11, "GetState(System.Threading.InternalThread)", ves_icall_System_Threading_Thread_GetState)) ICALL(THREAD_53, "InterruptInternal", ves_icall_System_Threading_Thread_Interrupt_internal) ICALL(THREAD_12, "JoinInternal", ves_icall_System_Threading_Thread_Join_internal) ICALL(THREAD_13, "MemoryBarrier", ves_icall_System_Threading_Thread_MemoryBarrier) @@ -950,7 +950,7 @@ ICALL(THREAD_14, "ResetAbortNative", ves_icall_System_Threading_Thread_ResetAbor ICALL(THREAD_15, "ResumeInternal", ves_icall_System_Threading_Thread_Resume) ICALL(THREAD_18, "SetName_internal(System.Threading.InternalThread,string)", ves_icall_System_Threading_Thread_SetName_internal) HANDLES(ICALL(THREAD_58, "SetPriorityNative", ves_icall_System_Threading_Thread_SetPriority)) -ICALL(THREAD_21, "SetState(System.Threading.InternalThread,System.Threading.ThreadState)", ves_icall_System_Threading_Thread_SetState) +HANDLES(ICALL(THREAD_21, "SetState(System.Threading.InternalThread,System.Threading.ThreadState)", ves_icall_System_Threading_Thread_SetState)) ICALL(THREAD_22, "SleepInternal", ves_icall_System_Threading_Thread_Sleep_internal) ICALL(THREAD_54, "SpinWait_nop", ves_icall_System_Threading_Thread_SpinWait_nop) ICALL(THREAD_23, "SuspendInternal", ves_icall_System_Threading_Thread_Suspend) diff --git a/mono/metadata/object-internals.h b/mono/metadata/object-internals.h index 431562913db..727be1cdd9a 100644 --- a/mono/metadata/object-internals.h +++ b/mono/metadata/object-internals.h @@ -438,12 +438,6 @@ struct _MonoInternalThread { gpointer last; }; -/* It's safe to access System.Threading.InternalThread from native code via a - * raw pointer because all instances should be pinned. But for uniformity of - * icall wrapping, let's declare a MonoInternalThreadHandle anyway. - */ -TYPED_HANDLE_DECL (MonoInternalThread); - struct _MonoThread { MonoObject obj; struct _MonoInternalThread *internal_thread; diff --git a/mono/metadata/threadpool.c b/mono/metadata/threadpool.c index 723be39f361..acf07d67656 100644 --- a/mono/metadata/threadpool.c +++ b/mono/metadata/threadpool.c @@ -355,9 +355,9 @@ worker_callback (void) mono_thread_set_name_internal (thread, thread_name, FALSE, TRUE, error); mono_error_assert_ok (error); - mono_thread_clr_state (thread, (MonoThreadState)~ThreadState_Background); - if (!mono_thread_test_state (thread , ThreadState_Background)) - ves_icall_System_Threading_Thread_SetState (thread, ThreadState_Background); + mono_thread_clear_and_set_state (thread, + (MonoThreadState)~ThreadState_Background, + ThreadState_Background); mono_thread_push_appdomain_ref (tpdomain->domain); if (mono_domain_set (tpdomain->domain, FALSE)) { diff --git a/mono/metadata/threads-types.h b/mono/metadata/threads-types.h index 92fdb245bcd..499276d2dc1 100644 --- a/mono/metadata/threads-types.h +++ b/mono/metadata/threads-types.h @@ -55,6 +55,12 @@ typedef enum { typedef struct _MonoInternalThread MonoInternalThread; +/* It's safe to access System.Threading.InternalThread from native code via a + * raw pointer because all instances should be pinned. But for uniformity of + * icall wrapping, let's declare a MonoInternalThreadHandle anyway. + */ +TYPED_HANDLE_DECL (MonoInternalThread); + typedef void (*MonoThreadCleanupFunc) (MonoNativeThreadId tid); /* INFO has type MonoThreadInfo* */ typedef void (*MonoThreadNotifyPendingExcFunc) (gpointer info); @@ -135,9 +141,9 @@ void ves_icall_System_Threading_Thread_ResetAbort (MonoThread *this_obj); MonoObject* ves_icall_System_Threading_Thread_GetAbortExceptionState (MonoThread *thread); void ves_icall_System_Threading_Thread_Suspend (MonoThread *this_obj); void ves_icall_System_Threading_Thread_Resume (MonoThread *thread); -void ves_icall_System_Threading_Thread_ClrState (MonoInternalThread *thread, guint32 state); -void ves_icall_System_Threading_Thread_SetState (MonoInternalThread *thread, guint32 state); -guint32 ves_icall_System_Threading_Thread_GetState (MonoInternalThread *thread); +void ves_icall_System_Threading_Thread_ClrState (MonoInternalThreadHandle thread, guint32 state, MonoError *error); +void ves_icall_System_Threading_Thread_SetState (MonoInternalThreadHandle thread_handle, guint32 state, MonoError *error); +guint32 ves_icall_System_Threading_Thread_GetState (MonoInternalThreadHandle thread_handle, MonoError *error); gint8 ves_icall_System_Threading_Thread_VolatileRead1 (void *ptr); gint16 ves_icall_System_Threading_Thread_VolatileRead2 (void *ptr); @@ -205,6 +211,7 @@ void mono_thread_set_state (MonoInternalThread *thread, MonoThreadState state); void mono_thread_clr_state (MonoInternalThread *thread, MonoThreadState state); gboolean mono_thread_test_state (MonoInternalThread *thread, MonoThreadState test); gboolean mono_thread_test_and_set_state (MonoInternalThread *thread, MonoThreadState test, MonoThreadState set); +void mono_thread_clear_and_set_state (MonoInternalThread *thread, MonoThreadState clear, MonoThreadState set); void mono_thread_init_apartment_state (void); void mono_thread_cleanup_apartment_state (void); diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c index 42f0c7ef12d..65dad823d21 100644 --- a/mono/metadata/threads.c +++ b/mono/metadata/threads.c @@ -2263,36 +2263,25 @@ ves_icall_System_Threading_Thread_MemoryBarrier (void) } void -ves_icall_System_Threading_Thread_ClrState (MonoInternalThread* this_obj, guint32 state) +ves_icall_System_Threading_Thread_ClrState (MonoInternalThreadHandle this_obj, guint32 state, MonoError *error) { - mono_thread_clr_state (this_obj, (MonoThreadState)state); - - if (state & ThreadState_Background) { - /* If the thread changes the background mode, the main thread has to - * be notified, since it has to rebuild the list of threads to - * wait for. - */ - mono_os_event_set (&background_change_event); - } + // InternalThreads are always pinned, so shallowly coop-handleize. + mono_thread_clr_state (mono_internal_thread_handle_ptr (this_obj), state); } void -ves_icall_System_Threading_Thread_SetState (MonoInternalThread* this_obj, guint32 state) +ves_icall_System_Threading_Thread_SetState (MonoInternalThreadHandle thread_handle, guint32 state, MonoError *error) { - mono_thread_set_state (this_obj, (MonoThreadState)state); - - if (state & ThreadState_Background) { - /* If the thread changes the background mode, the main thread has to - * be notified, since it has to rebuild the list of threads to - * wait for. - */ - mono_os_event_set (&background_change_event); - } + // InternalThreads are always pinned, so shallowly coop-handleize. + mono_thread_set_state (mono_internal_thread_handle_ptr (thread_handle), state); } guint32 -ves_icall_System_Threading_Thread_GetState (MonoInternalThread* this_obj) +ves_icall_System_Threading_Thread_GetState (MonoInternalThreadHandle thread_handle, MonoError *error) { + // InternalThreads are always pinned, so shallowly coop-handleize. + MonoInternalThread *this_obj = mono_internal_thread_handle_ptr (thread_handle); + guint32 state; LOCK_THREAD (this_obj); @@ -4751,12 +4740,37 @@ mono_thread_cleanup_apartment_state (void) #endif } +static void +mono_thread_notify_change_state (MonoThreadState old_state, MonoThreadState new_state) +{ + MonoThreadState diff = old_state ^ new_state; + if (diff & ThreadState_Background) { + /* If the thread changes the background mode, the main thread has to + * be notified, since it has to rebuild the list of threads to + * wait for. + */ + mono_os_event_set (&background_change_event); + } +} + void -mono_thread_set_state (MonoInternalThread *thread, MonoThreadState state) +mono_thread_clear_and_set_state (MonoInternalThread *thread, MonoThreadState clear, MonoThreadState set) { LOCK_THREAD (thread); - thread->state |= state; + + MonoThreadState const old_state = thread->state; + MonoThreadState const new_state = (old_state & ~clear) | set; + thread->state = new_state; + UNLOCK_THREAD (thread); + + mono_thread_notify_change_state (old_state, new_state); +} + +void +mono_thread_set_state (MonoInternalThread *thread, MonoThreadState state) +{ + mono_thread_clear_and_set_state (thread, 0, state); } /** @@ -4768,36 +4782,36 @@ gboolean mono_thread_test_and_set_state (MonoInternalThread *thread, MonoThreadState test, MonoThreadState set) { LOCK_THREAD (thread); + + MonoThreadState const old_state = thread->state; - if ((thread->state & test) != 0) { + if ((old_state & test) != 0) { UNLOCK_THREAD (thread); return FALSE; } - thread->state |= set; + MonoThreadState const new_state = old_state | set; + thread->state = new_state; + UNLOCK_THREAD (thread); + mono_thread_notify_change_state (old_state, new_state); + return TRUE; } void mono_thread_clr_state (MonoInternalThread *thread, MonoThreadState state) { - LOCK_THREAD (thread); - thread->state &= ~state; - UNLOCK_THREAD (thread); + mono_thread_clear_and_set_state (thread, state, 0); } gboolean mono_thread_test_state (MonoInternalThread *thread, MonoThreadState test) { - gboolean ret = FALSE; - LOCK_THREAD (thread); - if ((thread->state & test) != 0) { - ret = TRUE; - } + gboolean const ret = ((thread->state & test) != 0); UNLOCK_THREAD (thread); -- 2.11.4.GIT