Merge pull request #5504 from ntherning/wait-for-native-thread-to-die-in-Thread-Join
[mono.git] / mono / metadata / threads.c
index e0a7aee8edc4b6711420fb1961fd2373138bab09..3eb6b3b623c8994cf3b2442b79a4270c2eebcc0b 100644 (file)
@@ -45,6 +45,7 @@
 #include <mono/utils/mono-error-internals.h>
 #include <mono/utils/os-event.h>
 #include <mono/utils/mono-threads-debug.h>
+#include <mono/utils/unlocked.h>
 #include <mono/metadata/w32handle.h>
 #include <mono/metadata/w32event.h>
 #include <mono/metadata/w32mutex.h>
 #include <objbase.h>
 #endif
 
-#if defined(PLATFORM_ANDROID) && !defined(TARGET_ARM64) && !defined(TARGET_AMD64)
+#if defined(HOST_ANDROID) && !defined(TARGET_ARM64) && !defined(TARGET_AMD64)
 #define USE_TKILL_ON_ANDROID 1
 #endif
 
-#ifdef PLATFORM_ANDROID
+#ifdef HOST_ANDROID
 #include <errno.h>
 
 #ifdef USE_TKILL_ON_ANDROID
@@ -152,7 +153,7 @@ static MonoGHashTable *threads_starting_up = NULL;
 /* Contains tids */
 /* Protected by the threads lock */
 static GHashTable *joinable_threads;
-static int joinable_thread_count;
+static gint32 joinable_thread_count;
 
 #define SET_CURRENT_OBJECT(x) mono_tls_set_thread (x)
 #define GET_CURRENT_OBJECT() (MonoInternalThread*) mono_tls_get_thread ()
@@ -462,8 +463,8 @@ typedef union {
 #define SPECIAL_STATIC_OFFSET_TYPE_THREAD 0
 #define SPECIAL_STATIC_OFFSET_TYPE_CONTEXT 1
 
-#define MAKE_SPECIAL_STATIC_OFFSET(index, offset, type) \
-       ((SpecialStaticOffset) { .fields = { (index), (offset), (type) } }.raw)
+#define MAKE_SPECIAL_STATIC_OFFSET(idx, off, ty) \
+       ((SpecialStaticOffset) { .fields = { .index = (idx), .offset = (off), .type = (ty) } }.raw)
 #define ACCESS_SPECIAL_STATIC_OFFSET(x,f) \
        (((SpecialStaticOffset *) &(x))->fields.f)
 
@@ -737,6 +738,149 @@ mono_thread_attach_internal (MonoThread *thread, gboolean force_attach, gboolean
        return TRUE;
 }
 
+static void
+mono_thread_detach_internal (MonoInternalThread *thread)
+{
+       gboolean removed;
+
+       g_assert (thread != NULL);
+       SET_CURRENT_OBJECT (thread);
+
+       THREAD_DEBUG (g_message ("%s: mono_thread_detach for %p (%"G_GSIZE_FORMAT")", __func__, thread, (gsize)thread->tid));
+
+#ifndef HOST_WIN32
+       mono_w32mutex_abandon ();
+#endif
+
+       if (thread->abort_state_handle) {
+               mono_gchandle_free (thread->abort_state_handle);
+               thread->abort_state_handle = 0;
+       }
+
+       thread->abort_exc = NULL;
+       thread->current_appcontext = NULL;
+
+       /*
+        * thread->synch_cs can be NULL if this was called after
+        * ves_icall_System_Threading_InternalThread_Thread_free_internal.
+        * This can happen only during shutdown.
+        * The shutting_down flag is not always set, so we can't assert on it.
+        */
+       if (thread->synch_cs)
+               LOCK_THREAD (thread);
+
+       thread->state |= ThreadState_Stopped;
+       thread->state &= ~ThreadState_Background;
+
+       if (thread->synch_cs)
+               UNLOCK_THREAD (thread);
+
+       /*
+       An interruption request has leaked to cleanup. Adjust the global counter.
+
+       This can happen is the abort source thread finds the abortee (this) thread
+       in unmanaged code. If this thread never trips back to managed code or check
+       the local flag it will be left set and positively unbalance the global counter.
+
+       Leaving the counter unbalanced will cause a performance degradation since all threads
+       will now keep checking their local flags all the time.
+       */
+       mono_thread_clear_interruption_requested (thread);
+
+       mono_threads_lock ();
+
+       if (!threads) {
+               removed = FALSE;
+       } else if (mono_g_hash_table_lookup (threads, (gpointer)thread->tid) != thread) {
+               /* We have to check whether the thread object for the
+                * tid is still the same in the table because the
+                * thread might have been destroyed and the tid reused
+                * in the meantime, in which case the tid would be in
+                * the table, but with another thread object.
+                */
+               removed = FALSE;
+       } else {
+               mono_g_hash_table_remove (threads, (gpointer)thread->tid);
+               removed = TRUE;
+       }
+
+       mono_threads_unlock ();
+
+       /* Don't close the handle here, wait for the object finalizer
+        * to do it. Otherwise, the following race condition applies:
+        *
+        * 1) Thread exits (and mono_thread_detach_internal() closes the handle)
+        *
+        * 2) Some other handle is reassigned the same slot
+        *
+        * 3) Another thread tries to join the first thread, and
+        * blocks waiting for the reassigned handle to be signalled
+        * (which might never happen).  This is possible, because the
+        * thread calling Join() still has a reference to the first
+        * thread's object.
+        */
+
+       /* if the thread is not in the hash it has been removed already */
+       if (!removed) {
+               mono_domain_unset ();
+               mono_memory_barrier ();
+
+               if (mono_thread_cleanup_fn)
+                       mono_thread_cleanup_fn (thread_get_tid (thread));
+
+               goto done;
+       }
+
+       mono_release_type_locks (thread);
+
+       /* Can happen when we attach the profiler helper thread in order to heapshot. */
+       if (!mono_thread_info_lookup (MONO_UINT_TO_NATIVE_THREAD_ID (thread->tid))->tools_thread)
+               MONO_PROFILER_RAISE (thread_stopped, (thread->tid));
+
+       mono_hazard_pointer_clear (mono_hazard_pointer_get (), 1);
+
+       /*
+        * This will signal async signal handlers that the thread has exited.
+        * The profiler callback needs this to be set, so it cannot be done earlier.
+        */
+       mono_domain_unset ();
+       mono_memory_barrier ();
+
+       if (thread == mono_thread_internal_current ())
+               mono_thread_pop_appdomain_ref ();
+
+       mono_free_static_data (thread->static_data);
+       thread->static_data = NULL;
+       ref_stack_destroy (thread->appdomain_refs);
+       thread->appdomain_refs = NULL;
+
+       g_assert (thread->suspended);
+       mono_os_event_destroy (thread->suspended);
+       g_free (thread->suspended);
+       thread->suspended = NULL;
+
+       if (mono_thread_cleanup_fn)
+               mono_thread_cleanup_fn (thread_get_tid (thread));
+
+       mono_memory_barrier ();
+
+       if (mono_gc_is_moving ()) {
+               MONO_GC_UNREGISTER_ROOT (thread->thread_pinning_ref);
+               thread->thread_pinning_ref = NULL;
+       }
+
+done:
+       SET_CURRENT_OBJECT (NULL);
+       mono_domain_unset ();
+
+       mono_thread_info_unset_internal_thread_gchandle ((MonoThreadInfo*) thread->thread_info);
+
+       /* Don't need to close the handle to this thread, even though we took a
+        * reference in mono_thread_attach (), because the GC will do it
+        * when the Thread object is finalised.
+        */
+}
+
 typedef struct {
        gint32 ref;
        MonoThread *thread;
@@ -1138,149 +1282,6 @@ mono_thread_attach (MonoDomain *domain)
        return mono_thread_attach_full (domain, FALSE);
 }
 
-void
-mono_thread_detach_internal (MonoInternalThread *thread)
-{
-       gboolean removed;
-
-       g_assert (thread != NULL);
-       SET_CURRENT_OBJECT (thread);
-
-       THREAD_DEBUG (g_message ("%s: mono_thread_detach for %p (%"G_GSIZE_FORMAT")", __func__, thread, (gsize)thread->tid));
-
-#ifndef HOST_WIN32
-       mono_w32mutex_abandon ();
-#endif
-
-       if (thread->abort_state_handle) {
-               mono_gchandle_free (thread->abort_state_handle);
-               thread->abort_state_handle = 0;
-       }
-
-       thread->abort_exc = NULL;
-       thread->current_appcontext = NULL;
-
-       /*
-        * thread->synch_cs can be NULL if this was called after
-        * ves_icall_System_Threading_InternalThread_Thread_free_internal.
-        * This can happen only during shutdown.
-        * The shutting_down flag is not always set, so we can't assert on it.
-        */
-       if (thread->synch_cs)
-               LOCK_THREAD (thread);
-
-       thread->state |= ThreadState_Stopped;
-       thread->state &= ~ThreadState_Background;
-
-       if (thread->synch_cs)
-               UNLOCK_THREAD (thread);
-
-       /*
-       An interruption request has leaked to cleanup. Adjust the global counter.
-
-       This can happen is the abort source thread finds the abortee (this) thread
-       in unmanaged code. If this thread never trips back to managed code or check
-       the local flag it will be left set and positively unbalance the global counter.
-       
-       Leaving the counter unbalanced will cause a performance degradation since all threads
-       will now keep checking their local flags all the time.
-       */
-       mono_thread_clear_interruption_requested (thread);
-
-       mono_threads_lock ();
-
-       if (!threads) {
-               removed = FALSE;
-       } else if (mono_g_hash_table_lookup (threads, (gpointer)thread->tid) != thread) {
-               /* We have to check whether the thread object for the
-                * tid is still the same in the table because the
-                * thread might have been destroyed and the tid reused
-                * in the meantime, in which case the tid would be in
-                * the table, but with another thread object.
-                */
-               removed = FALSE;
-       } else {
-               mono_g_hash_table_remove (threads, (gpointer)thread->tid);
-               removed = TRUE;
-       }
-
-       mono_threads_unlock ();
-
-       /* Don't close the handle here, wait for the object finalizer
-        * to do it. Otherwise, the following race condition applies:
-        *
-        * 1) Thread exits (and mono_thread_detach_internal() closes the handle)
-        *
-        * 2) Some other handle is reassigned the same slot
-        *
-        * 3) Another thread tries to join the first thread, and
-        * blocks waiting for the reassigned handle to be signalled
-        * (which might never happen).  This is possible, because the
-        * thread calling Join() still has a reference to the first
-        * thread's object.
-        */
-
-       /* if the thread is not in the hash it has been removed already */
-       if (!removed) {
-               mono_domain_unset ();
-               mono_memory_barrier ();
-
-               if (mono_thread_cleanup_fn)
-                       mono_thread_cleanup_fn (thread_get_tid (thread));
-
-               goto done;
-       }
-
-       mono_release_type_locks (thread);
-
-       /* Can happen when we attach the profiler helper thread in order to heapshot. */
-       if (!mono_thread_info_lookup (MONO_UINT_TO_NATIVE_THREAD_ID (thread->tid))->tools_thread)
-               MONO_PROFILER_RAISE (thread_stopped, (thread->tid));
-
-       mono_hazard_pointer_clear (mono_hazard_pointer_get (), 1);
-
-       /*
-        * This will signal async signal handlers that the thread has exited.
-        * The profiler callback needs this to be set, so it cannot be done earlier.
-        */
-       mono_domain_unset ();
-       mono_memory_barrier ();
-
-       if (thread == mono_thread_internal_current ())
-               mono_thread_pop_appdomain_ref ();
-
-       mono_free_static_data (thread->static_data);
-       thread->static_data = NULL;
-       ref_stack_destroy (thread->appdomain_refs);
-       thread->appdomain_refs = NULL;
-
-       g_assert (thread->suspended);
-       mono_os_event_destroy (thread->suspended);
-       g_free (thread->suspended);
-       thread->suspended = NULL;
-
-       if (mono_thread_cleanup_fn)
-               mono_thread_cleanup_fn (thread_get_tid (thread));
-
-       mono_memory_barrier ();
-
-       if (mono_gc_is_moving ()) {
-               MONO_GC_UNREGISTER_ROOT (thread->thread_pinning_ref);
-               thread->thread_pinning_ref = NULL;
-       }
-
-done:
-       SET_CURRENT_OBJECT (NULL);
-       mono_domain_unset ();
-
-       mono_thread_info_unset_internal_thread_gchandle ((MonoThreadInfo*) thread->thread_info);
-
-       /* Don't need to close the handle to this thread, even though we took a
-        * reference in mono_thread_attach (), because the GC will do it
-        * when the Thread object is finalised.
-        */
-}
-
 /**
  * mono_thread_detach:
  */
@@ -1801,7 +1802,7 @@ mono_join_uninterrupted (MonoThreadHandle* thread_to_join, gint32 ms, MonoError
 }
 
 gboolean
-ves_icall_System_Threading_Thread_Join_internal(MonoThread *this_obj, int ms)
+ves_icall_System_Threading_Thread_Join_internal (MonoThread *this_obj, int ms)
 {
        MonoInternalThread *thread = this_obj->internal_thread;
        MonoThreadHandle *handle = thread->handle;
@@ -1821,30 +1822,42 @@ ves_icall_System_Threading_Thread_Join_internal(MonoThread *this_obj, int ms)
                return FALSE;
        }
 
+       MonoNativeThreadId tid = thread_get_tid (thread);
+
        UNLOCK_THREAD (thread);
 
-       if(ms== -1) {
-               ms=MONO_INFINITE_WAIT;
-       }
+       if (ms == -1)
+               ms = MONO_INFINITE_WAIT;
        THREAD_DEBUG (g_message ("%s: joining thread handle %p, %d ms", __func__, handle, ms));
-       
+
        mono_thread_set_state (cur_thread, ThreadState_WaitSleepJoin);
 
-       ret=mono_join_uninterrupted (handle, ms, &error);
+       ret = mono_join_uninterrupted (handle, ms, &error);
 
        mono_thread_clr_state (cur_thread, ThreadState_WaitSleepJoin);
 
        mono_error_set_pending_exception (&error);
 
-       if(ret==MONO_THREAD_INFO_WAIT_RET_SUCCESS_0) {
+       if (ret == MONO_THREAD_INFO_WAIT_RET_SUCCESS_0) {
                THREAD_DEBUG (g_message ("%s: join successful", __func__));
 
-               return(TRUE);
+#ifdef HOST_WIN32
+               /* TODO: Do this on Unix platforms as well. See PR #5454 for context.  */
+               /* Wait for the thread to really exit */
+               MONO_ENTER_GC_SAFE;
+               /* This shouldn't block */
+               mono_threads_join_lock ();
+               mono_native_thread_join (tid);
+               mono_threads_join_unlock ();
+               MONO_EXIT_GC_SAFE;
+#endif
+
+               return TRUE;
        }
        
        THREAD_DEBUG (g_message ("%s: join failed", __func__));
 
-       return(FALSE);
+       return FALSE;
 }
 
 #define MANAGED_WAIT_FAILED 0x7fffffff
@@ -3036,6 +3049,8 @@ mono_thread_callbacks_init (void)
 void
 mono_thread_cleanup (void)
 {
+       mono_threads_join_threads ();
+
 #if !defined(RUN_IN_SUBTHREAD) && !defined(HOST_WIN32)
        /* The main thread must abandon any held mutexes (particularly
         * important for named mutexes as they are shared across
@@ -3947,6 +3962,14 @@ mono_threads_abort_appdomain_threads (MonoDomain *domain, int timeout)
        return TRUE;
 }
 
+void
+mono_thread_self_abort (void)
+{
+       MonoError error;
+       self_abort_internal (&error);
+       mono_error_set_pending_exception (&error);
+}
+
 /*
  * mono_thread_get_undeniable_exception:
  *
@@ -4992,7 +5015,7 @@ mono_threads_add_joinable_thread (gpointer tid)
        if (!joinable_threads)
                joinable_threads = g_hash_table_new (NULL, NULL);
        g_hash_table_insert (joinable_threads, tid, tid);
-       joinable_thread_count ++;
+       UnlockedIncrement (&joinable_thread_count);
        joinable_threads_unlock ();
 
        mono_gc_finalize_notify ();
@@ -5016,7 +5039,7 @@ mono_threads_join_threads (void)
        gboolean found;
 
        /* Fastpath */
-       if (!joinable_thread_count)
+       if (!UnlockedRead (&joinable_thread_count))
                return;
 
        while (TRUE) {
@@ -5027,7 +5050,7 @@ mono_threads_join_threads (void)
                        g_hash_table_iter_next (&iter, &key, (void**)&tid);
                        thread = (pthread_t)tid;
                        g_hash_table_remove (joinable_threads, key);
-                       joinable_thread_count --;
+                       UnlockedDecrement (&joinable_thread_count);
                        found = TRUE;
                }
                joinable_threads_unlock ();
@@ -5065,7 +5088,7 @@ mono_thread_join (gpointer tid)
                joinable_threads = g_hash_table_new (NULL, NULL);
        if (g_hash_table_lookup (joinable_threads, tid)) {
                g_hash_table_remove (joinable_threads, tid);
-               joinable_thread_count --;
+               UnlockedDecrement (&joinable_thread_count);
                found = TRUE;
        }
        joinable_threads_unlock ();