[threads] Signal the w32handle later in the thread destruction process (#3652)
authorLudovic Henry <ludovic@xamarin.com>
Tue, 27 Sep 2016 21:44:51 +0000 (23:44 +0200)
committerGitHub <noreply@github.com>
Tue, 27 Sep 2016 21:44:51 +0000 (23:44 +0200)
mono/utils/mono-threads-posix.c
mono/utils/mono-threads-windows.c
mono/utils/mono-threads.c
mono/utils/mono-threads.h

index a8a5b53274e95a0041f8ca6cbed2510df8229c40..9e6ebe712c2c446ba607190f6d4fb8d14c477c62 100644 (file)
@@ -157,7 +157,11 @@ mono_threads_platform_exit (int exit_code)
 void
 mono_threads_platform_unregister (MonoThreadInfo *info)
 {
-       mono_threads_platform_set_exited (info);
+       g_assert (info->handle);
+
+       /* The thread is no longer active, so unref it */
+       mono_w32handle_unref (info->handle);
+       info->handle = NULL;
 }
 
 int
@@ -285,28 +289,23 @@ mono_native_thread_set_name (MonoNativeThreadId tid, const char *name)
 }
 
 void
-mono_threads_platform_set_exited (MonoThreadInfo *info)
+mono_threads_platform_set_exited (gpointer handle)
 {
        int thr_ret;
 
-       g_assert (info->handle);
-       if (mono_w32handle_issignalled (info->handle))
-               g_error ("%s: handle %p thread %p has already exited, it's handle is signalled", __func__, info->handle, mono_thread_info_get_tid (info));
-       if (mono_w32handle_get_type (info->handle) == MONO_W32HANDLE_UNUSED)
-               g_error ("%s: handle %p thread %p has already exited, it's handle type is 'unused'", __func__, info->handle, mono_thread_info_get_tid (info));
+       g_assert (handle);
+       if (mono_w32handle_issignalled (handle))
+               g_error ("%s: handle %p thread %p has already exited, it's handle is signalled", __func__, handle, mono_native_thread_id_get ());
+       if (mono_w32handle_get_type (handle) == MONO_W32HANDLE_UNUSED)
+               g_error ("%s: handle %p thread %p has already exited, it's handle type is 'unused'", __func__, handle, mono_native_thread_id_get ());
 
-       thr_ret = mono_w32handle_lock_handle (info->handle);
+       thr_ret = mono_w32handle_lock_handle (handle);
        g_assert (thr_ret == 0);
 
-       mono_w32handle_set_signal_state (info->handle, TRUE, TRUE);
+       mono_w32handle_set_signal_state (handle, TRUE, TRUE);
 
-       thr_ret = mono_w32handle_unlock_handle (info->handle);
+       thr_ret = mono_w32handle_unlock_handle (handle);
        g_assert (thr_ret == 0);
-
-       /* The thread is no longer active, so unref it */
-       mono_w32handle_unref (info->handle);
-
-       info->handle = NULL;
 }
 
 static const gchar* thread_typename (void)
index e2b489ecc1064f6eea5fc9c1bd76818232ef1941..7beb557fc70ef3661f0c5b7e0ed7705dd97708b1 100644 (file)
@@ -243,7 +243,10 @@ mono_threads_platform_exit (int exit_code)
 void
 mono_threads_platform_unregister (MonoThreadInfo *info)
 {
-       mono_threads_platform_set_exited (info);
+       g_assert (info->handle);
+
+       CloseHandle (info->handle);
+       info->handle = NULL;
 }
 
 int
@@ -309,12 +312,8 @@ mono_native_thread_set_name (MonoNativeThreadId tid, const char *name)
 }
 
 void
-mono_threads_platform_set_exited (MonoThreadInfo *info)
+mono_threads_platform_set_exited (gpointer handle)
 {
-       g_assert (info->handle);
-       // No need to call CloseHandle() here since the InternalThread
-       // destructor will close the handle when the finalizer thread calls it
-       info->handle = NULL;
 }
 
 void
index 3eb88171257a0f68687870c9655144791ae779d9..f8c716b5038d941b9a74edbf0f0ab84e52be9450 100644 (file)
@@ -399,6 +399,7 @@ unregister_thread (void *arg)
        MonoThreadInfo *info;
        int small_id;
        gboolean result;
+       gpointer handle;
 
        info = (MonoThreadInfo *) arg;
        g_assert (info);
@@ -423,6 +424,10 @@ unregister_thread (void *arg)
        mono_native_tls_set_value (small_id_key, GUINT_TO_POINTER (info->small_id + 1));
 #endif
 
+       /* we need to duplicate it, as the info->handle is going
+        * to be closed when unregistering from the platform */
+       handle = mono_threads_platform_duplicate_handle (info);
+
        /*
        First perform the callback that requires no locks.
        This callback has the potential of taking other locks, so we do it before.
@@ -457,6 +462,13 @@ unregister_thread (void *arg)
        mono_thread_hazardous_try_free_some ();
 
        mono_thread_small_id_free (small_id);
+
+       /* Signal the w32handle. It can be done as late as here
+        * because w32handle does not access the current MonoThreadInfo,
+        * neither does it switch state to BLOCKING. */
+       mono_threads_platform_set_exited (handle);
+
+       mono_threads_platform_close_thread_handle (handle);
 }
 
 static void
@@ -1638,7 +1650,9 @@ void
 mono_thread_info_set_exited (THREAD_INFO_TYPE *info)
 {
        g_assert (mono_thread_info_is_current (info));
-       mono_threads_platform_set_exited (info);
+
+       g_assert (info->handle);
+       mono_threads_platform_set_exited (info->handle);
 }
 
 gpointer
index db4bdab73b73dee37082ed8fbdf610a64c4902b7..5e31607c176eb466536b4a38db672d4efcd54e0d 100644 (file)
@@ -522,7 +522,7 @@ gboolean mono_threads_platform_yield (void);
 void mono_threads_platform_exit (int exit_code);
 HANDLE mono_threads_platform_open_thread_handle (HANDLE handle, MonoNativeThreadId tid);
 void mono_threads_platform_close_thread_handle (HANDLE handle);
-void mono_threads_platform_set_exited (THREAD_INFO_TYPE *info);
+void mono_threads_platform_set_exited (gpointer handle);
 gpointer mono_threads_platform_duplicate_handle (THREAD_INFO_TYPE *info);
 
 void mono_threads_coop_begin_global_suspend (void);