[threads] Switch foreign threads to GC Safe in mono_thread_detach (#42758)
authormonojenkins <jo.shields+jenkins@xamarin.com>
Thu, 8 Oct 2020 14:31:28 +0000 (10:31 -0400)
committerGitHub <noreply@github.com>
Thu, 8 Oct 2020 14:31:28 +0000 (10:31 -0400)
* [test] Invoke from foreign threads then try to GC and shutdown

   Even if the non-Mono threads that once called Mono are looping or deadlocked,
   they shouldn't prevent us from doing a GC (in hybrid suspend mode) or from
   shutting down if they detached from the runtime.

* [tests] update .gitignore

   Ignore AOT build artifacts in subdirectories of mono/tests/, too

* [threads] Make mono_thread_attach external only

   Runtime should use mono_thread_internal_attach

* [threads] Mark mono_thread_detach external only; switch to GC Safe

   Runtime threads should call mono_thread_internal_detach

   Addresses https://github.com/mono/mono/issues/20290 and https://github.com/mono/mono/issues/20283

   If a foreign thread (that was created outside the runtime) calls
   mono_thread_detach, leave it in a preemptively-suspendable state, since we
   can't expect it to coop suspend.

   Conversely in mono_thread_attach (external only), ensure that we always leave
   the thread in GC Unsafe (aka RUNNING) state, for cases like

       while (cond) {
         t = mono_thread_attach (domain);
         <...>
         mono_thread_detach (t);
       }

* Tests fixup

   Delete test that invokes the runtime in a loop forever.
   This is just exercising a race between mono_thread_attach and the
   runtime shutdown.  Instead update the invoke_foreign_thread test to
   loop a few times to check that attach/detach loops are ok.

  In the deadlock test, wait for the foreign thread to finish calling
  the runtime before the test thread returns from native back to managed
   to avoid a race between shutdown and the invoke.

* [interp] Set context to null when freeing

   If a foreign thread runs this loop

       for (...) {
         mono_thread_attach;
         mono_runtime_invoke;
         mono_thread_detach;
      }

   on the second iteration it will get a ThreadContext that was already freed
   during the detach.  Set the TLS variable to null before freeing the context.

* [threads] Switch to GC Unsafe before creating managed thread object

   For a re-attaching move the thread state transition to happen earlier so that
   create_internal_thread_object (which does a managed allocation) is always done
   in GC Unsafe mode.

* fixup eventpipe to use mono_thread_internal_{attach,detach}

Co-authored-by: lambdageek <lambdageek@users.noreply.github.com>
Co-authored-by: Aleksey Kliger <alklig@microsoft.com>
15 files changed:
src/mono/mono/eventpipe/ep-rt-mono.h
src/mono/mono/metadata/appdomain.c
src/mono/mono/metadata/cominterop.c
src/mono/mono/metadata/icall-eventpipe.c
src/mono/mono/metadata/threads-types.h
src/mono/mono/metadata/threads.c
src/mono/mono/metadata/threads.h
src/mono/mono/mini/interp/interp.c
src/mono/mono/mini/mini-runtime.c
src/mono/mono/profiler/aot.c
src/mono/mono/profiler/log.c
src/mono/mono/tests/.gitignore
src/mono/mono/tests/Makefile.am
src/mono/mono/tests/libtest.c
src/mono/mono/tests/pinvoke-detach-1.cs [new file with mode: 0644]

index d2d18fd..48c67d3 100644 (file)
@@ -1528,7 +1528,7 @@ ep_rt_thread_setup (bool background_thread)
 #ifdef EP_RT_MONO_USE_STATIC_RUNTIME
        // NOTE, under netcore, only root domain exists.
        if (!mono_thread_current ()) {
-               MonoThread *thread = mono_thread_attach (mono_get_root_domain ());
+               MonoThread *thread = mono_thread_internal_attach (mono_get_root_domain ());
                if (background_thread && thread) {
                        mono_thread_set_state (thread, ThreadState_Background);
                        mono_thread_info_set_flags (MONO_THREAD_INFO_FLAGS_NO_SAMPLE);
@@ -1547,7 +1547,7 @@ ep_rt_thread_teardown (void)
 #ifdef EP_RT_MONO_USE_STATIC_RUNTIME
        MonoThread *current_thread = mono_thread_current ();
        if (current_thread)
-               mono_thread_detach (current_thread);
+               mono_thread_internal_detach (current_thread);
 #else
        ep_rt_mono_func_table_get ()->ep_rt_mono_thread_detach ();
 #endif
index 685b127..397045a 100644 (file)
@@ -349,7 +349,7 @@ mono_runtime_init_checked (MonoDomain *domain, MonoThreadStartCB start_cb, MonoT
                domain->setup = MONO_HANDLE_RAW (setup);
        }
 
-       mono_thread_attach (domain);
+       mono_thread_internal_attach (domain);
 
 #if defined(ENABLE_PERFTRACING) && !defined(DISABLE_EVENTPIPE)
        ds_server_init ();
index d3219a8..ec7227c 100644 (file)
@@ -2859,7 +2859,7 @@ cominterop_ccw_queryinterface_impl (MonoCCWInterface* ccwe, const guint8* riid,
                *ppv = NULL;
 
        if (!mono_domain_get ())
-               mono_thread_attach (mono_get_root_domain ());
+               mono_thread_attach_external_native_thread (mono_get_root_domain (), FALSE);
 
        /* handle IUnknown special */
        if (cominterop_class_guid_equal (riid, mono_class_get_iunknown_class ())) {
@@ -2990,7 +2990,7 @@ cominterop_ccw_get_ids_of_names_impl (MonoCCWInterface* ccwe, gpointer riid,
        klass = mono_object_class (object);
 
        if (!mono_domain_get ())
-                mono_thread_attach (mono_get_root_domain ());
+               mono_thread_attach_external_native_thread (mono_get_root_domain (), FALSE);
 
        for (i=0; i < cNames; i++) {
                methodname = mono_unicode_to_external (rgszNames[i]);
index 1588220..e6961c6 100644 (file)
@@ -113,7 +113,7 @@ eventpipe_thread_attach (gboolean background_thread)
 
        // NOTE, under netcore, only root domain exists.
        if (!mono_thread_current ()) {
-               thread = mono_thread_attach (mono_get_root_domain ());
+               thread = mono_thread_internal_attach (mono_get_root_domain ());
                if (background_thread) {
                        mono_thread_set_state (thread, ThreadState_Background);
                        mono_thread_info_set_flags (MONO_THREAD_INFO_FLAGS_NO_SAMPLE);
@@ -129,7 +129,7 @@ eventpipe_thread_detach (void)
 {
        MonoThread *current_thread = mono_thread_current ();
        if (current_thread)
-               mono_thread_detach (current_thread);
+               mono_thread_internal_detach (current_thread);
 }
 
 void
index dc06fb5..ab72d65 100644 (file)
@@ -425,6 +425,16 @@ void mono_threads_add_joinable_thread (gpointer tid);
 void mono_threads_join_threads (void);
 void mono_thread_join (gpointer tid);
 
+MONO_PROFILER_API MonoThread*
+mono_thread_internal_attach (MonoDomain *domain);
+
+MONO_PROFILER_API void
+mono_thread_internal_detach (MonoThread *thread);
+
+MonoThread *
+mono_thread_attach_external_native_thread (MonoDomain *domain, gboolean background);
+
+
 MONO_API gpointer
 mono_threads_attach_coop (MonoDomain *domain, gpointer *dummy);
 
index e37dd79..362cd30 100644 (file)
@@ -702,6 +702,8 @@ init_internal_thread_object (MonoInternalThread *thread)
 static MonoInternalThread*
 create_internal_thread_object (void)
 {
+       MONO_REQ_GC_UNSAFE_MODE;
+
        ERROR_DECL (error);
        MonoInternalThread *thread;
        MonoVTable *vt;
@@ -1526,6 +1528,58 @@ mono_thread_create_checked (MonoDomain *domain, gpointer func, gpointer arg, Mon
 MonoThread *
 mono_thread_attach (MonoDomain *domain)
 {
+       return mono_thread_attach_external_native_thread (domain, FALSE);
+}
+
+/**
+ * mono_thread_attach_external_native_thread:
+ *
+ * Attach the current thread (that was created outside the runtime or managed
+ * code) to the runtime.  If \p background is TRUE, set the IsBackground
+ * property on the thread.
+ *
+ * COOP: On return, the thread is in GC Unsafe mode
+ */
+MonoThread *
+mono_thread_attach_external_native_thread (MonoDomain *domain, gboolean background)
+{
+       MonoThread *thread = mono_thread_internal_attach (domain);
+
+       if (background)
+               mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
+
+#if 0
+       /* Can't do this - would break embedders who do their own GC thread
+        * state transitions.  Also while the conversion of MONO_API entry
+        * points to do a transition to GC Unsafe is not complete, doing a
+        * transition here potentially means running runtime code while in GC
+        * Safe mode.
+        */
+       if (mono_threads_is_blocking_transition_enabled ()) {
+               /* mono_jit_thread_attach and mono_thread_attach are external-only and
+                * not called by the runtime on any of our own threads.  So if we get
+                * here, the thread is running native code - leave it in GC Safe mode
+                * and leave it to the n2m invoke wrappers or MONO_API entry points to
+                * switch to GC Unsafe.
+                */
+               MONO_STACKDATA (stackdata);
+               mono_threads_enter_gc_safe_region_unbalanced_internal (&stackdata);
+       }
+#endif
+       return thread;
+}
+
+/**
+ * mono_thread_internal_attach:
+ *
+ * Attach the current thread to the runtime.  The thread was created on behalf
+ * of the runtime and the runtime is responsible for it.
+ *
+ * COOP: On return, the thread is in GC Unsafe mode
+ */
+MonoThread *
+mono_thread_internal_attach (MonoDomain *domain)
+{
        MonoInternalThread *internal;
        MonoThread *thread;
        MonoThreadInfo *info;
@@ -1538,7 +1592,34 @@ mono_thread_attach (MonoDomain *domain)
                return mono_thread_current ();
        }
 
-       info = mono_thread_info_attach ();
+       if (G_UNLIKELY ((info = mono_thread_info_current_unchecked ()))) {
+               /* 
+                * We are not attached currently, but we were earlier.  Ensure the thread is in GC Unsafe mode.
+                * Have to do this before creating the managed thread object.
+                *
+                */
+               if (mono_threads_is_blocking_transition_enabled ()) {
+                       /*
+                        * Ensure the thread is in RUNNING state.
+                        * If the thread is doing something like this
+                        *
+                        * while (cond) {
+                        *   t = mono_thread_attach (domain);
+                        *   <...>
+                        *   mono_thread_detach (t);
+                        * }
+                        *
+                        * The call to mono_thread_detach will put it in GC Safe
+                        * (blocking, preemptive suspend) mode, so the next time we
+                        * come back to attach, we need to switch to GC Unsafe
+                        * (running, cooperative suspend) mode.
+                        */
+                       MONO_STACKDATA (stackdata);
+                       mono_threads_enter_gc_unsafe_region_unbalanced_internal (&stackdata);
+               }
+       } else {
+               info = mono_thread_info_attach ();
+       }
        g_assert (info);
 
        tid=mono_native_thread_id_get ();
@@ -1596,12 +1677,39 @@ mono_threads_attach_tools_thread (void)
 
 /**
  * mono_thread_detach:
+ *
+ * COOP: On return, the thread is in GC Safe mode
  */
 void
 mono_thread_detach (MonoThread *thread)
 {
-       if (thread)
-               mono_thread_detach_internal (thread->internal_thread);
+       if (!thread)
+               return;
+       mono_thread_internal_detach (thread);
+       /*
+        * If the thread wasn't created by the runtime, leave it in GC
+        * Safe mode.  Under hybrid and coop suspend, we don't want to
+        * wait for it to cooperatively suspend.
+        */
+       if (mono_threads_is_blocking_transition_enabled ()) {
+               MONO_STACKDATA (stackdata);
+               mono_threads_enter_gc_safe_region_unbalanced_internal (&stackdata);
+       }
+}
+
+/**
+ * mono_thread_internal_detach:
+ *
+ * COOP: GC thread state is unchanged
+ */
+void
+mono_thread_internal_detach (MonoThread *thread)
+{
+       if (!thread)
+               return;
+       MONO_ENTER_GC_UNSAFE;
+       mono_thread_detach_internal (thread->internal_thread);
+       MONO_EXIT_GC_UNSAFE;
 }
 
 
@@ -3789,6 +3897,8 @@ mono_thread_manage_internal (void)
         * Also abort all the background threads
         * */
        do {
+               THREAD_DEBUG (g_message ("%s: abort phase", __func__));
+
                mono_threads_lock ();
 
                wait->num = 0;
@@ -6074,7 +6184,7 @@ mono_threads_attach_coop_internal (MonoDomain *domain, gpointer *cookie, MonoSta
                external = !(info = mono_thread_info_current_unchecked ()) || !mono_thread_info_is_live (info);
 
        if (!mono_thread_internal_current ()) {
-               mono_thread_attach (domain);
+               mono_thread_internal_attach (domain);
 
                // #678164
                mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
@@ -6082,7 +6192,7 @@ mono_threads_attach_coop_internal (MonoDomain *domain, gpointer *cookie, MonoSta
 
        if (mono_threads_is_blocking_transition_enabled ()) {
                if (external) {
-                       /* mono_thread_attach put the thread in RUNNING mode from STARTING, but we need to
+                       /* mono_thread_internal_attach put the thread in RUNNING mode from STARTING, but we need to
                         * return the right cookie. */
                        *cookie = mono_threads_enter_gc_unsafe_region_cookie ();
                } else {
index b1b9103..89be80f 100644 (file)
@@ -40,8 +40,10 @@ MONO_API void mono_thread_new_init (intptr_t tid, void* stack_start,
 MONO_API MONO_RT_EXTERNAL_ONLY void
 mono_thread_create (MonoDomain *domain, void* func, void* arg);
 
-MONO_API MonoThread *mono_thread_attach (MonoDomain *domain);
-MONO_API void mono_thread_detach (MonoThread *thread);
+MONO_API MONO_RT_EXTERNAL_ONLY MonoThread *
+mono_thread_attach (MonoDomain *domain);
+MONO_API MONO_RT_EXTERNAL_ONLY void
+mono_thread_detach (MonoThread *thread);
 MONO_API void mono_thread_exit (void);
 
 MONO_API MONO_RT_EXTERNAL_ONLY void
index e985c60..96f9925 100644 (file)
@@ -407,6 +407,14 @@ interp_free_context (gpointer ctx)
 {
        ThreadContext *context = (ThreadContext*)ctx;
 
+       ThreadContext *current_context = (ThreadContext *) mono_native_tls_get_value (thread_context_id);
+       /* at thread exit, we can be called from the JIT TLS key destructor with current_context == NULL */
+       if (current_context != NULL) {
+               /* check that the context we're freeing is the current one before overwriting TLS */
+               g_assert (context == current_context);
+               set_context (NULL);
+       }
+
        mono_vfree (context->stack_start, INTERP_STACK_SIZE, MONO_MEM_ACCOUNT_INTERP_STACK);
        /* Prevent interp_mark_stack from trying to scan the data_stack, before freeing it */
        context->stack_start = NULL;
index c8cc526..4459b86 100644 (file)
@@ -866,10 +866,9 @@ mono_jit_thread_attach (MonoDomain *domain)
        attached = mono_tls_get_jit_tls () != NULL;
 
        if (!attached) {
-               mono_thread_attach (domain);
-
                // #678164
-               mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
+               gboolean background = TRUE;
+               mono_thread_attach_external_native_thread (domain, background);
 
                /* mono_jit_thread_attach is external-only and not called by
                 * the runtime on any of our own threads.  So if we get here,
@@ -4649,7 +4648,7 @@ mini_init (const char *filename, const char *runtime_version)
        mono_install_runtime_cleanup (runtime_cleanup);
        mono_runtime_init_checked (domain, (MonoThreadStartCB)mono_thread_start_cb, mono_thread_attach_cb, error);
        mono_error_assert_ok (error);
-       mono_thread_attach (domain);
+       mono_thread_internal_attach (domain);
        MONO_PROFILER_RAISE (thread_name, (MONO_NATIVE_THREAD_ID_TO_UINT (mono_native_thread_id_get ()), "Main"));
 #endif
        mono_threads_set_runtime_startup_finished ();
index d919dda..2c4c267 100644 (file)
@@ -213,7 +213,7 @@ static void prof_save (MonoProfiler *prof, FILE* file);
 static void *
 helper_thread (void *arg)
 {
-       mono_thread_attach (mono_get_root_domain ());
+       mono_thread_internal_attach (mono_get_root_domain ());
 
        mono_thread_set_name_constant_ignore_error (mono_thread_internal_current (), "AOT Profiler Helper", MonoSetThreadNameFlag_None);
 
@@ -312,7 +312,7 @@ helper_thread (void *arg)
        prof_shutdown (&aot_profiler);
 
        mono_thread_info_set_flags (MONO_THREAD_INFO_FLAGS_NONE);
-       mono_thread_detach (mono_thread_current ());
+       mono_thread_internal_detach (mono_thread_current ());
 
        return NULL;
 }
index 2e70b20..440b1ff 100644 (file)
@@ -3150,7 +3150,7 @@ profiler_thread_begin_function (const char *name8, const gunichar2* name16, size
        mono_thread_info_attach ();
        MonoProfilerThread *thread = init_thread (FALSE);
 
-       mono_thread_attach (mono_get_root_domain ());
+       mono_thread_internal_attach (mono_get_root_domain ());
 
        MonoInternalThread *internal = mono_thread_internal_current ();
 
@@ -3199,7 +3199,7 @@ profiler_thread_check_detach (MonoProfilerThread *thread)
                thread->did_detach = TRUE;
 
                mono_thread_info_set_flags (MONO_THREAD_INFO_FLAGS_NONE);
-               mono_thread_detach (mono_thread_current ());
+               mono_thread_internal_detach (mono_thread_current ());
 
                mono_os_sem_post (&log_profiler.detach_threads_sem);
        }
index 835205a..d4b5120 100644 (file)
@@ -19,7 +19,7 @@
 /*.pdb
 /*.o
 /*.lo
-/*.so
+**.so
 **.dylib
 **.dylib.dSYM
 /*.netmodule
index 8b689e9..fc33d5c 100755 (executable)
@@ -342,6 +342,7 @@ TESTS_CS_SRC=               \
        pinvoke11.cs            \
        pinvoke13.cs            \
        pinvoke17.cs            \
+       pinvoke-detach-1.cs     \
        invoke.cs               \
        invoke2.cs              \
        runtime-invoke.cs               \
index 1e624ea..c2e3639 100644 (file)
@@ -7789,6 +7789,13 @@ mono_test_native_to_managed_exception_rethrow (NativeToManagedExceptionRethrowFu
 typedef void (*VoidVoidCallback) (void);
 typedef void (*MonoFtnPtrEHCallback) (guint32 gchandle);
 
+typedef void *MonoDomain;
+typedef void *MonoAssembly;
+typedef void *MonoImage;
+typedef void *MonoClass;
+typedef void *MonoMethod;
+typedef void *MonoThread;
+
 typedef long long MonoObject;
 typedef MonoObject MonoException;
 typedef int32_t mono_bool;
@@ -7803,25 +7810,60 @@ static void (*sym_mono_domain_unload) (gpointer);
 static void (*sym_mono_threads_exit_gc_safe_region_unbalanced) (gpointer, gpointer *);
 static void (*null_function_ptr) (void);
 
-static void
-mono_test_init_symbols (void)
-{
-       if (sym_inited)
-               return;
+static MonoDomain *(*sym_mono_get_root_domain) (void);
+
+static MonoDomain *(*sym_mono_domain_get)(void);
+
+static mono_bool (*sym_mono_domain_set)(MonoDomain *, mono_bool /*force */);
+
+static MonoAssembly *(*sym_mono_domain_assembly_open) (MonoDomain *, const char*);
+
+static MonoImage *(*sym_mono_assembly_get_image) (MonoAssembly *);
 
-       sym_mono_install_ftnptr_eh_callback = (void (*) (MonoFtnPtrEHCallback)) (lookup_mono_symbol ("mono_install_ftnptr_eh_callback"));
+static MonoClass *(*sym_mono_class_from_name)(MonoImage *, const char *, const char *);
 
-       sym_mono_gchandle_get_target = (MonoObject* (*) (guint32 gchandle)) (lookup_mono_symbol ("mono_gchandle_get_target"));
+static MonoMethod *(*sym_mono_class_get_method_from_name)(MonoClass *, const char *, int /* arg_count */);
 
-       sym_mono_gchandle_new = (guint32 (*) (MonoObject *, mono_bool)) (lookup_mono_symbol ("mono_gchandle_new"));
+static MonoThread *(*sym_mono_thread_attach)(MonoDomain *);
 
-       sym_mono_gchandle_free = (void (*) (guint32 gchandle)) (lookup_mono_symbol ("mono_gchandle_free"));
+static void (*sym_mono_thread_detach)(MonoThread *);
 
-       sym_mono_raise_exception = (void (*) (MonoException *)) (lookup_mono_symbol ("mono_raise_exception"));
+static MonoObject *(*sym_mono_runtime_invoke) (MonoMethod *, void*, void**, MonoObject**);
 
-       sym_mono_domain_unload = (void (*) (gpointer)) (lookup_mono_symbol ("mono_domain_unload"));
 
-       sym_mono_threads_exit_gc_safe_region_unbalanced = (void (*) (gpointer, gpointer *)) (lookup_mono_symbol ("mono_threads_exit_gc_safe_region_unbalanced"));
+// SYM_LOOKUP(mono_runtime_invoke)
+// expands to
+//  sym_mono_runtime_invoke = g_cast (lookup_mono_symbol ("mono_runtime_invoke"));
+//
+// (the g_cast is necessary for C++ builds)
+#define SYM_LOOKUP(name) do {                  \
+       sym_##name = g_cast (lookup_mono_symbol (#name));       \
+       } while (0)
+
+static void
+mono_test_init_symbols (void)
+{
+       if (sym_inited)
+               return;
+
+       SYM_LOOKUP (mono_install_ftnptr_eh_callback);
+       SYM_LOOKUP (mono_gchandle_get_target);
+       SYM_LOOKUP (mono_gchandle_new);
+       SYM_LOOKUP (mono_gchandle_free);
+       SYM_LOOKUP (mono_raise_exception);
+       SYM_LOOKUP (mono_domain_unload);
+       SYM_LOOKUP (mono_threads_exit_gc_safe_region_unbalanced);
+
+       SYM_LOOKUP (mono_get_root_domain);
+       SYM_LOOKUP (mono_domain_get);
+       SYM_LOOKUP (mono_domain_set);
+       SYM_LOOKUP (mono_domain_assembly_open);
+       SYM_LOOKUP (mono_assembly_get_image);
+       SYM_LOOKUP (mono_class_from_name);
+       SYM_LOOKUP (mono_class_get_method_from_name);
+       SYM_LOOKUP (mono_thread_attach);
+       SYM_LOOKUP (mono_thread_detach);
+       SYM_LOOKUP (mono_runtime_invoke);
 
        sym_inited = 1;
 }
@@ -8259,6 +8301,155 @@ mono_test_marshal_return_array (void)
        return static_arr;
 }
 
+struct invoke_names {
+       char *assm_name;
+       char *name_space;
+       char *name;
+       char *meth_name;
+};
+
+static struct invoke_names *
+make_invoke_names (const char *assm_name, const char *name_space, const char *name, const char *meth_name)
+{
+       struct invoke_names *names = (struct invoke_names*) malloc (sizeof (struct invoke_names));
+       names->assm_name = strdup (assm_name);
+       names->name_space = strdup (name_space);
+       names->name = strdup (name);
+       names->meth_name = strdup (meth_name);
+       return names;
+}
+
+static void
+destroy_invoke_names (struct invoke_names *n)
+{
+       free (n->assm_name);
+       free (n->name_space);
+       free (n->name);
+       free (n->meth_name);
+       free (n);
+}
+
+static void
+test_invoke_by_name (struct invoke_names *names)
+{
+       mono_test_init_symbols ();
+
+       MonoDomain *domain = sym_mono_domain_get ();
+       MonoThread *thread = NULL;
+       if (!domain) {
+               thread = sym_mono_thread_attach (sym_mono_get_root_domain ());
+       }
+       domain = sym_mono_domain_get ();
+       g_assert (domain);
+       MonoAssembly *assm = sym_mono_domain_assembly_open (domain, names->assm_name);
+       g_assert (assm);
+       MonoImage *image = sym_mono_assembly_get_image (assm);
+       MonoClass *klass = sym_mono_class_from_name (image, names->name_space, names->name);
+       g_assert (klass);
+       /* meth_name should be a static method that takes no arguments */
+       MonoMethod *method = sym_mono_class_get_method_from_name (klass, names->meth_name, -1);
+       g_assert (method);
+
+       MonoObject *args[] = {NULL, };
+
+       sym_mono_runtime_invoke (method, NULL, (void**)args, NULL);
+
+       if (thread)
+               sym_mono_thread_detach (thread);
+}
+
+#ifndef HOST_WIN32
+static void*
+invoke_foreign_thread (void* user_data)
+{
+       struct invoke_names *names = (struct invoke_names*)user_data;
+        /*
+         * Run a couple of times to check that attach/detach multiple
+         * times from the same thread leaves it in a reasonable coop
+         * thread state.
+         */
+        for (int i = 0; i < 5; ++i) {
+                test_invoke_by_name (names);
+                sleep (2);
+        }
+       destroy_invoke_names (names);
+       return NULL;
+}
+#endif
+
+
+LIBTEST_API mono_bool STDCALL
+mono_test_attach_invoke_foreign_thread (const char *assm_name, const char *name_space, const char *name, const char *meth_name)
+{
+#ifndef HOST_WIN32
+       struct invoke_names *names = make_invoke_names (assm_name, name_space, name, meth_name);
+       pthread_t t;
+       int res = pthread_create (&t, NULL, invoke_foreign_thread, (void*)names);
+       g_assert (res == 0);
+       pthread_join (t, NULL);
+       return 0;
+#else
+       // TODO: Win32 version of this test
+       return 1;
+#endif
+}
+
+#ifndef HOST_WIN32
+struct names_and_mutex {
+       struct invoke_names *names;
+        /* mutex to coordinate test and foreign thread */
+        pthread_mutex_t coord_mutex;
+        pthread_cond_t coord_cond;
+        /* mutex to block the foreign thread */
+       pthread_mutex_t deadlock_mutex;
+};
+
+static void*
+invoke_block_foreign_thread (void *user_data)
+{
+       // This thread calls into the runtime and then blocks. It should not
+       // prevent the runtime from shutting down.
+       struct names_and_mutex *nm = (struct names_and_mutex *)user_data;
+       test_invoke_by_name (nm->names);
+        pthread_mutex_lock (&nm->coord_mutex);
+        /* signal the test thread that we called the runtime */
+        pthread_cond_signal (&nm->coord_cond);
+        pthread_mutex_unlock (&nm->coord_mutex);
+
+       pthread_mutex_lock (&nm->deadlock_mutex); // blocks forever
+       g_assert_not_reached ();
+}
+#endif
+
+LIBTEST_API mono_bool STDCALL
+mono_test_attach_invoke_block_foreign_thread (const char *assm_name, const char *name_space, const char *name, const char *meth_name)
+{
+#ifndef HOST_WIN32
+       struct invoke_names *names = make_invoke_names (assm_name, name_space, name, meth_name);
+       struct names_and_mutex *nm = malloc (sizeof (struct names_and_mutex));
+       nm->names = names;
+        pthread_mutex_init (&nm->coord_mutex, NULL);
+        pthread_cond_init (&nm->coord_cond, NULL);
+       pthread_mutex_init (&nm->deadlock_mutex, NULL);
+
+       pthread_mutex_lock (&nm->deadlock_mutex); // lock the mutex and never unlock it.
+       pthread_t t;
+       int res = pthread_create (&t, NULL, invoke_block_foreign_thread, (void*)nm);
+       g_assert (res == 0);
+        /* wait for the foreign thread to finish calling the runtime before
+         * detaching it and returning
+         */
+        pthread_mutex_lock (&nm->coord_mutex);
+        pthread_cond_wait (&nm->coord_cond, &nm->coord_mutex);
+        pthread_mutex_unlock (&nm->coord_mutex);
+       pthread_detach (t);
+       return 0;
+#else
+       // TODO: Win32 version of this test
+       return 1;
+#endif
+}
+
 #ifdef __cplusplus
 } // extern C
 #endif
diff --git a/src/mono/mono/tests/pinvoke-detach-1.cs b/src/mono/mono/tests/pinvoke-detach-1.cs
new file mode 100644 (file)
index 0000000..5b475fd
--- /dev/null
@@ -0,0 +1,56 @@
+//
+// pinvoke-detach-1.cs:
+//
+//   Test attaching and detaching a new thread from native.
+//  If everything is working, this should not hang on shutdown.
+using System;
+using System.Threading;
+using System.Runtime.InteropServices;
+
+public class MonoPInvokeCallbackAttribute : Attribute {
+       public MonoPInvokeCallbackAttribute (Type delegateType) { }
+}
+
+public class Tests {
+       public static int Main ()
+       {
+               return TestDriver.RunTests (typeof (Tests));
+       }
+
+       public delegate void VoidVoidDelegate ();
+
+       static int was_called;
+
+       [MonoPInvokeCallback (typeof (VoidVoidDelegate))]
+       private static void MethodInvokedFromNative ()
+       {
+               was_called++;
+       }
+
+       [DllImport ("libtest", EntryPoint="mono_test_attach_invoke_foreign_thread")]
+       public static extern bool mono_test_attach_invoke_foreign_thread (string assm_name, string name_space, string class_name, string method_name);
+
+       public static int test_0_attach_invoke_foreign_thread ()
+       {
+               was_called = 0;
+               bool skipped = mono_test_attach_invoke_foreign_thread (typeof (Tests).Assembly.Location, "", "Tests", "MethodInvokedFromNative");
+               return skipped || was_called == 5 ? 0 : 1;
+       }
+
+       [MonoPInvokeCallback (typeof (VoidVoidDelegate))]
+       private static void MethodInvokedFromNative2 ()
+       {
+       }
+
+       [DllImport ("libtest", EntryPoint="mono_test_attach_invoke_block_foreign_thread")]
+       public static extern bool mono_test_attach_invoke_block_foreign_thread (string assm_name, string name_space, string class_name, string method_name);
+
+       public static int test_0_attach_invoke_block_foreign_thread ()
+       {
+               bool skipped = mono_test_attach_invoke_block_foreign_thread (typeof (Tests).Assembly.Location, "", "Tests", "MethodInvokedFromNative2");
+               GC.Collect (); // should not hang waiting for the foreign thread
+               return 0; // really we succeed if the app can shut down without hanging
+       }
+
+
+}