[threads] Detach thread in all possible cases (#3584)
authorLudovic Henry <ludovic@xamarin.com>
Tue, 20 Sep 2016 15:12:37 +0000 (17:12 +0200)
committerGitHub <noreply@github.com>
Tue, 20 Sep 2016 15:12:37 +0000 (17:12 +0200)
* [threads] Use mono_thread_detach_internal to factor code

* [threads] Make sure we detach the thread before exiting

* [threads] Add mono_thread_internal_current_is_attached

* [threads] Ensure we detach the thread on Boehm

mono/metadata/boehm-gc.c
mono/metadata/sgen-mono.c
mono/metadata/threads-types.h
mono/metadata/threads.c

index d3153d29967deb995f5d80da007197e6f7575475..d22f162085b376b1a0d7a64412e82acd8454b588 100644 (file)
@@ -58,6 +58,8 @@ boehm_thread_register (MonoThreadInfo* info, void *baseptr);
 static void
 boehm_thread_unregister (MonoThreadInfo *p);
 static void
+boehm_thread_detach (MonoThreadInfo *p);
+static void
 register_test_toggleref_callback (void);
 
 #define BOEHM_GC_BIT_FINALIZER_AWARE 1
@@ -236,6 +238,7 @@ mono_gc_base_init (void)
        memset (&cb, 0, sizeof (cb));
        cb.thread_register = boehm_thread_register;
        cb.thread_unregister = boehm_thread_unregister;
+       cb.thread_detach = boehm_thread_detach;
        cb.mono_method_is_critical = (gboolean (*)(void *))mono_runtime_is_critical_method;
 
        mono_threads_init (&cb, sizeof (MonoThreadInfo));
@@ -406,6 +409,13 @@ boehm_thread_unregister (MonoThreadInfo *p)
                mono_threads_add_joinable_thread ((gpointer)tid);
 }
 
+static void
+boehm_thread_detach (MonoThreadInfo *p)
+{
+       if (mono_thread_internal_current_is_attached ())
+               mono_thread_detach_internal (mono_thread_internal_current ());
+}
+
 gboolean
 mono_object_is_alive (MonoObject* o)
 {
index c902fcdbe5a92ad3aa5427b21c4bceed9ce42e0a..92b6f0686feac05ece21f9993bdc85a185109c7f 100644 (file)
@@ -2301,7 +2301,7 @@ sgen_thread_detach (SgenThreadInfo *p)
         * so we assume that if the domain is still registered, we can detach
         * the thread
         */
-       if (mono_domain_get ())
+       if (mono_thread_internal_current_is_attached ())
                mono_thread_detach_internal (mono_thread_internal_current ());
 }
 
index ad472a0b9c06030dcc1e3cba4a235bc9c6287517..9da0dfc6c54eaeae50e985008b330b565bb1ab94 100644 (file)
@@ -260,4 +260,7 @@ void mono_threads_begin_abort_protected_block (void);
 void mono_threads_end_abort_protected_block (void);
 MonoException* mono_thread_try_resume_interruption (void);
 
+gboolean
+mono_thread_internal_current_is_attached (void);
+
 #endif /* _MONO_METADATA_THREADS_TYPES_H_ */
index 8d8510f5fb0edca44e597ccf064b725d2ea02ffc..1965c3b4254ee32dbfe89d9b06750d1c14a958ef 100644 (file)
@@ -874,20 +874,10 @@ static guint32 WINAPI start_wrapper_internal(StartInfo *start_info, gsize *stack
         * for the current thead */
        mono_thread_cleanup_apartment_state ();
 
-       thread_cleanup (internal);
+       mono_thread_detach_internal (internal);
 
        internal->tid = 0;
 
-       /* Remove the reference to the thread object in the TLS data,
-        * so the thread object can be finalized.  This won't be
-        * reached if the thread threw an uncaught exception, so those
-        * thread handles will stay referenced :-( (This is due to
-        * missing support for scanning thread-specific data in the
-        * Boehm GC - the io-layer keeps a GC-visible hash of pointers
-        * to TLS data.)
-        */
-       SET_CURRENT_OBJECT (NULL);
-
        return(0);
 }
 
@@ -1075,7 +1065,7 @@ mono_thread_attach_full (MonoDomain *domain, gboolean force_attach)
        MonoNativeThreadId tid;
        gsize stack_ptr;
 
-       if ((internal = mono_thread_internal_current ())) {
+       if (mono_thread_internal_current_is_attached ()) {
                if (domain != mono_domain_get ())
                        mono_domain_set (domain, TRUE);
                /* Already attached */
@@ -1168,6 +1158,18 @@ mono_thread_detach_if_exiting (void)
        return FALSE;
 }
 
+gboolean
+mono_thread_internal_current_is_attached (void)
+{
+       MonoInternalThread *internal;
+
+       internal = GET_CURRENT_OBJECT ();
+       if (!internal)
+               return FALSE;
+
+       return TRUE;
+}
+
 void
 mono_thread_exit (void)
 {
@@ -1175,13 +1177,12 @@ mono_thread_exit (void)
 
        THREAD_DEBUG (g_message ("%s: mono_thread_exit for %p (%"G_GSIZE_FORMAT")", __func__, thread, (gsize)thread->tid));
 
-       thread_cleanup (thread);
-       SET_CURRENT_OBJECT (NULL);
-       mono_domain_unset ();
+       mono_thread_detach_internal (thread);
 
        /* we could add a callback here for embedders to use. */
        if (mono_thread_get_main () && (thread == mono_thread_get_main ()->internal_thread))
                exit (mono_environment_exitcode_get ());
+
        mono_thread_info_exit ();
 }
 
@@ -3122,8 +3123,8 @@ mono_threads_set_shutting_down (void)
                        UNLOCK_THREAD (current_thread);
                }
 
-               /*since we're killing the thread, unset the current domain.*/
-               mono_domain_unset ();
+               /*since we're killing the thread, detach it.*/
+               mono_thread_detach_internal (current_thread);
 
                /* Wake up other threads potentially waiting for us */
                mono_thread_info_exit ();