Merge pull request #2593 from ludovic-henry/coop-fix-native-to-managed
authorLudovic Henry <ludovic@xamarin.com>
Mon, 22 Feb 2016 11:23:34 +0000 (11:23 +0000)
committerLudovic Henry <ludovic@xamarin.com>
Mon, 22 Feb 2016 11:23:34 +0000 (11:23 +0000)
[coop] Fix native-to-managed wrapper

mono/metadata/marshal.c
mono/mini/debugger-agent.c
mono/mini/method-to-ir.c
mono/mini/mini-runtime.c
mono/mini/mini.h
msvc/mono.def
msvc/monosgen.def

index 0c04c03b684cf0b6ec21d9f48b19cc7022eb730e..82e0b9b69fb17d61e6f69e1cc8fed412c2c1e2db 100644 (file)
@@ -7754,9 +7754,10 @@ mono_marshal_emit_managed_wrapper (MonoMethodBuilder *mb, MonoMethodSignature *i
        }
 #else
        MonoMethodSignature *sig, *csig;
+       MonoExceptionClause *clause;
        int i, *tmp_locals;
+       int leave_pos;
        gboolean closed = FALSE;
-       int coop_gc_var, coop_gc_dummy_local;
 
        sig = m->sig;
        csig = m->csig;
@@ -7783,30 +7784,46 @@ mono_marshal_emit_managed_wrapper (MonoMethodBuilder *mb, MonoMethodSignature *i
                mono_mb_add_local (mb, sig->ret);
        }
 
+       /*
+        * try {
+        *   mono_jit_attach ();
+        *
+        *   <interrupt check>
+        *
+        *   ret = method (...);
+        * } finally {
+        *   mono_jit_detach ();
+        * }
+        *
+        * return ret;
+        */
+
        if (mono_threads_is_coop_enabled ()) {
-               /* local 4, the local to be used when calling the reset_blocking funcs */
-               /* tons of code hardcode 3 to be the return var */
-               coop_gc_var = mono_mb_add_local (mb, &mono_defaults.int_class->byval_arg);
-               /* local 5, the local used to get a stack address for suspend funcs */
-               coop_gc_dummy_local = mono_mb_add_local (mb, &mono_defaults.int_class->byval_arg);
+               clause = g_new0 (MonoExceptionClause, 1);
+               clause->flags = MONO_EXCEPTION_CLAUSE_FINALLY;
        }
 
        mono_mb_emit_icon (mb, 0);
        mono_mb_emit_stloc (mb, 2);
 
+       if (mono_threads_is_coop_enabled ()) {
+               /* try { */
+               clause->try_offset = mono_mb_get_label (mb);
+       }
+
        /*
         * Might need to attach the thread to the JIT or change the
         * domain for the callback.
+        *
+        * Also does the (STARTING|BLOCKING|RUNNING) -> RUNNING thread state transtion
+        *
+        * mono_jit_attach ();
         */
        mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
        mono_mb_emit_byte (mb, CEE_MONO_JIT_ATTACH);
 
-       if (mono_threads_is_coop_enabled ()) {
-               /* XXX can we merge reset_blocking_start with JIT_ATTACH above and save one call? */
-               mono_mb_emit_ldloc_addr (mb, coop_gc_dummy_local);
-               mono_mb_emit_icall (mb, mono_threads_reset_blocking_start);
-               mono_mb_emit_stloc (mb, coop_gc_var);
-       }
+       /* <interrupt check> */
+       emit_thread_interrupt_checkpoint (mb);
 
        /* we first do all conversions */
        tmp_locals = (int *)alloca (sizeof (int) * sig->param_count);
@@ -7830,8 +7847,6 @@ mono_marshal_emit_managed_wrapper (MonoMethodBuilder *mb, MonoMethodSignature *i
                }
        }
 
-       emit_thread_interrupt_checkpoint (mb);
-
        if (sig->hasthis) {
                if (target_handle) {
                        mono_mb_emit_icon (mb, (gint32)target_handle);
@@ -7858,6 +7873,7 @@ mono_marshal_emit_managed_wrapper (MonoMethodBuilder *mb, MonoMethodSignature *i
                        mono_mb_emit_ldarg (mb, i);
        }
 
+       /* ret = method (...) */
        mono_mb_emit_managed_call (mb, method, NULL);
 
        if (mspecs [0] && mspecs [0]->native == MONO_NATIVE_CUSTOM) {
@@ -7937,15 +7953,31 @@ mono_marshal_emit_managed_wrapper (MonoMethodBuilder *mb, MonoMethodSignature *i
        }
 
        if (mono_threads_is_coop_enabled ()) {
-               /* XXX merge reset_blocking_end with detach */
-               mono_mb_emit_ldloc (mb, coop_gc_var);
-               mono_mb_emit_ldloc_addr (mb, coop_gc_dummy_local);
-               mono_mb_emit_icall (mb, mono_threads_reset_blocking_end);
+               leave_pos = mono_mb_emit_branch (mb, CEE_LEAVE);
+
+               /* } finally { */
+               clause->try_len = mono_mb_get_label (mb) - clause->try_offset;
+               clause->handler_offset = mono_mb_get_label (mb);
        }
 
+       /*
+        * Also does the RUNNING -> (BLOCKING|RUNNING) thread state transition
+        *
+        * mono_jit_detach ();
+        */
        mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
        mono_mb_emit_byte (mb, CEE_MONO_JIT_DETACH);
 
+       if (mono_threads_is_coop_enabled ()) {
+               mono_mb_emit_byte (mb, CEE_ENDFINALLY);
+
+               /* } [endfinally] */
+               clause->handler_len = mono_mb_get_pos (mb) - clause->handler_offset;
+
+               mono_mb_patch_branch (mb, leave_pos);
+       }
+
+       /* return ret; */
        if (m->retobj_var) {
                mono_mb_emit_ldloc (mb, m->retobj_var);
                mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
@@ -7957,6 +7989,10 @@ mono_marshal_emit_managed_wrapper (MonoMethodBuilder *mb, MonoMethodSignature *i
                mono_mb_emit_byte (mb, CEE_RET);
        }
 
+       if (mono_threads_is_coop_enabled ()) {
+               mono_mb_set_clauses (mb, 1, clause);
+       }
+
        if (closed)
                g_free (sig);
 #endif
index 88acd40395bb82e7bacc2003731e47c11674766a..6b72d9571d8497909aa7fcb721f557d7b4ee70c3 100644 (file)
@@ -9630,12 +9630,13 @@ debugger_thread (void *arg)
        ErrorCode err;
        gboolean no_reply;
        gboolean attach_failed = FALSE;
+       gpointer attach_cookie, attach_dummy;
 
        DEBUG_PRINTF (1, "[dbg] Agent thread started, pid=%p\n", (gpointer) (gsize) mono_native_thread_id_get ());
 
        debugger_thread_id = mono_native_thread_id_get ();
 
-       mono_jit_thread_attach (mono_get_root_domain ());
+       attach_cookie = mono_jit_thread_attach (mono_get_root_domain (), &attach_dummy);
 
        mono_thread_internal_current ()->flags |= MONO_THREAD_FLAG_DONT_MANAGE;
 
@@ -9788,7 +9789,9 @@ debugger_thread (void *arg)
                DEBUG_PRINTF (2, "[dbg] Detached - restarting clean debugger thread.\n");
                start_debugger_thread ();
        }
-       
+
+       mono_jit_thread_detach (attach_cookie, &attach_dummy);
+
        return 0;
 }
 
index 159686b1f61d8ed942591921dfcccf4bc2c07680..d76f2085b9697f5d8576adc580146a57017a426c 100644 (file)
@@ -12751,47 +12751,54 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
                                MonoInst *ad_ins, *jit_tls_ins;
                                MonoBasicBlock *next_bb = NULL, *call_bb = NULL;
 
-                               cfg->orig_domain_var = mono_compile_create_var (cfg, &mono_defaults.int_class->byval_arg, OP_LOCAL);
+                               cfg->attach_cookie = mono_compile_create_var (cfg, &mono_defaults.int_class->byval_arg, OP_LOCAL);
+                               cfg->attach_dummy = mono_compile_create_var (cfg, &mono_defaults.int_class->byval_arg, OP_LOCAL);
 
-                               EMIT_NEW_PCONST (cfg, ins, NULL);
-                               MONO_EMIT_NEW_UNALU (cfg, OP_MOVE, cfg->orig_domain_var->dreg, ins->dreg);
+                               if (mono_threads_is_coop_enabled ()) {
+                                       /* AOT code is only used in the root domain */
+                                       EMIT_NEW_PCONST (cfg, args [0], cfg->compile_aot ? NULL : cfg->domain);
+                                       EMIT_NEW_VARLOADA (cfg, args [1], cfg->attach_dummy, cfg->attach_dummy->inst_vtype);
+                                       ins = mono_emit_jit_icall (cfg, mono_jit_thread_attach, args);
+                                       MONO_EMIT_NEW_UNALU (cfg, OP_MOVE, cfg->attach_cookie->dreg, ins->dreg);
+                               } else {
+                                       EMIT_NEW_PCONST (cfg, ins, NULL);
+                                       MONO_EMIT_NEW_UNALU (cfg, OP_MOVE, cfg->attach_cookie->dreg, ins->dreg);
 
-                               ad_ins = mono_get_domain_intrinsic (cfg);
-                               jit_tls_ins = mono_get_jit_tls_intrinsic (cfg);
+                                       ad_ins = mono_get_domain_intrinsic (cfg);
+                                       jit_tls_ins = mono_get_jit_tls_intrinsic (cfg);
 
-                               if (cfg->backend->have_tls_get && ad_ins && jit_tls_ins) {
-                                       NEW_BBLOCK (cfg, next_bb);
-                                       NEW_BBLOCK (cfg, call_bb);
+                                       if (cfg->backend->have_tls_get && ad_ins && jit_tls_ins) {
+                                               NEW_BBLOCK (cfg, next_bb);
+                                               NEW_BBLOCK (cfg, call_bb);
 
-                                       if (cfg->compile_aot) {
-                                               /* AOT code is only used in the root domain */
-                                               EMIT_NEW_PCONST (cfg, domain_ins, NULL);
-                                       } else {
-                                               EMIT_NEW_PCONST (cfg, domain_ins, cfg->domain);
-                                       }
-                                       MONO_ADD_INS (cfg->cbb, ad_ins);
-                                       MONO_EMIT_NEW_BIALU (cfg, OP_COMPARE, -1, ad_ins->dreg, domain_ins->dreg);
-                                       MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBNE_UN, call_bb);
+                                               if (cfg->compile_aot) {
+                                                       /* AOT code is only used in the root domain */
+                                                       EMIT_NEW_PCONST (cfg, domain_ins, NULL);
+                                               } else {
+                                                       EMIT_NEW_PCONST (cfg, domain_ins, cfg->domain);
+                                               }
+                                               MONO_ADD_INS (cfg->cbb, ad_ins);
+                                               MONO_EMIT_NEW_BIALU (cfg, OP_COMPARE, -1, ad_ins->dreg, domain_ins->dreg);
+                                               MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBNE_UN, call_bb);
 
-                                       MONO_ADD_INS (cfg->cbb, jit_tls_ins);
-                                       MONO_EMIT_NEW_BIALU_IMM (cfg, OP_COMPARE_IMM, -1, jit_tls_ins->dreg, 0);
-                                       MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBEQ, call_bb);
+                                               MONO_ADD_INS (cfg->cbb, jit_tls_ins);
+                                               MONO_EMIT_NEW_BIALU_IMM (cfg, OP_COMPARE_IMM, -1, jit_tls_ins->dreg, 0);
+                                               MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBEQ, call_bb);
 
-                                       MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_BR, next_bb);
-                                       MONO_START_BB (cfg, call_bb);
-                               }
+                                               MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_BR, next_bb);
+                                               MONO_START_BB (cfg, call_bb);
+                                       }
 
-                               if (cfg->compile_aot) {
                                        /* AOT code is only used in the root domain */
-                                       EMIT_NEW_PCONST (cfg, args [0], NULL);
-                               } else {
-                                       EMIT_NEW_PCONST (cfg, args [0], cfg->domain);
+                                       EMIT_NEW_PCONST (cfg, args [0], cfg->compile_aot ? NULL : cfg->domain);
+                                       EMIT_NEW_PCONST (cfg, args [1], NULL);
+                                       ins = mono_emit_jit_icall (cfg, mono_jit_thread_attach, args);
+                                       MONO_EMIT_NEW_UNALU (cfg, OP_MOVE, cfg->attach_cookie->dreg, ins->dreg);
+
+                                       if (next_bb)
+                                               MONO_START_BB (cfg, next_bb);
                                }
-                               ins = mono_emit_jit_icall (cfg, mono_jit_thread_attach, args);
-                               MONO_EMIT_NEW_UNALU (cfg, OP_MOVE, cfg->orig_domain_var->dreg, ins->dreg);
 
-                               if (next_bb)
-                                       MONO_START_BB (cfg, next_bb);
                                ip += 2;
                                break;
                        }
@@ -12800,8 +12807,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 
                                /* Restore the original domain */
                                dreg = alloc_ireg (cfg);
-                               EMIT_NEW_UNALU (cfg, args [0], OP_MOVE, dreg, cfg->orig_domain_var->dreg);
-                               mono_emit_jit_icall (cfg, mono_jit_set_domain, args);
+                               EMIT_NEW_UNALU (cfg, args [0], OP_MOVE, dreg, cfg->attach_cookie->dreg);
+                               EMIT_NEW_VARLOADA (cfg, args [1], cfg->attach_dummy, cfg->attach_dummy->inst_vtype);
+                               mono_emit_jit_icall (cfg, mono_jit_thread_detach, args);
                                ip += 2;
                                break;
                        }
index 4a4161dfc30705460a95980da34a765847d4c321..a3fe3305d5c476af87670d5925414a5f628f8a3f 100644 (file)
@@ -825,7 +825,8 @@ mono_get_lmf_addr (void)
         * mono_get_lmf_addr, and mono_get_lmf_addr requires the thread to be attached.
         */
 
-       mono_jit_thread_attach (NULL);
+       mono_thread_attach (mono_get_root_domain ());
+       mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
 
        if ((jit_tls = mono_native_tls_get_value (mono_jit_tls_id)))
                return &jit_tls->lmf;
@@ -884,48 +885,119 @@ mono_set_lmf_addr (gpointer lmf_addr)
 }
 
 /*
- * mono_jit_thread_attach:
+ * mono_jit_thread_attach: called by native->managed wrappers
  *
- * Called by native->managed wrappers. Returns the original domain which needs to be
- * restored, or NULL.
+ * In non-coop mode:
+ *  - @dummy: is NULL
+ *  - @return: the original domain which needs to be restored, or NULL.
+ *
+ * In coop mode:
+ *  - @dummy: contains the original domain
+ *  - @return: a cookie containing current MonoThreadInfo* if it was in BLOCKING mode, NULL otherwise
  */
-MonoDomain*
-mono_jit_thread_attach (MonoDomain *domain)
+gpointer
+mono_jit_thread_attach (MonoDomain *domain, gpointer *dummy)
 {
        MonoDomain *orig;
 
-       if (!domain)
-               /*
-                * Happens when called from AOTed code which is only used in the root
-                * domain.
-                */
+       if (!domain) {
+               /* Happens when called from AOTed code which is only used in the root domain. */
                domain = mono_get_root_domain ();
+       }
+
+       g_assert (domain);
+
+       if (!mono_threads_is_coop_enabled ()) {
+               gboolean attached;
 
 #ifdef MONO_HAVE_FAST_TLS
-       if (!MONO_FAST_TLS_GET (mono_lmf_addr)) {
-               mono_thread_attach (domain);
-               // #678164
-               mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
-       }
+               attached = MONO_FAST_TLS_GET (mono_lmf_addr) != NULL;
 #else
-       if (!mono_native_tls_get_value (mono_jit_tls_id)) {
-               mono_thread_attach (domain);
-               mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
-       }
+               attached = mono_native_tls_get_value (mono_jit_tls_id) != NULL;
 #endif
-       orig = mono_domain_get ();
-       if (orig != domain)
-               mono_domain_set (domain, TRUE);
 
-       return orig != domain ? orig : NULL;
+               if (!attached) {
+                       mono_thread_attach (domain);
+
+                       // #678164
+                       mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
+               }
+
+               orig = mono_domain_get ();
+               if (orig != domain)
+                       mono_domain_set (domain, TRUE);
+
+               return orig != domain ? orig : NULL;
+       } else {
+               MonoThreadInfo *info;
+
+               info = mono_thread_info_current_unchecked ();
+               if (!info || !mono_thread_info_is_live (info)) {
+                       /* thread state STARTING -> RUNNING */
+                       mono_thread_attach (domain);
+
+                       // #678164
+                       mono_thread_set_state (mono_thread_internal_current (), ThreadState_Background);
+
+                       *dummy = NULL;
+
+                       /* mono_threads_reset_blocking_start returns the current MonoThreadInfo
+                        * if we were in BLOCKING mode */
+                       return mono_thread_info_current ();
+               } else {
+                       orig = mono_domain_get ();
+
+                       /* orig might be null if we did an attach -> detach -> attach sequence */
+
+                       if (orig != domain)
+                               mono_domain_set (domain, TRUE);
+
+                       *dummy = orig;
+
+                       /* thread state (BLOCKING|RUNNING) -> RUNNING */
+                       return mono_threads_reset_blocking_start (dummy);
+               }
+       }
 }
 
-/* Called by native->managed wrappers */
+/*
+ * mono_jit_thread_detach: called by native->managed wrappers
+ *
+ * In non-coop mode:
+ *  - @cookie: the original domain which needs to be restored, or NULL.
+ *  - @dummy: is NULL
+ *
+ * In coop mode:
+ *  - @cookie: contains current MonoThreadInfo* if it was in BLOCKING mode, NULL otherwise
+ *  - @dummy: contains the original domain
+ */
 void
-mono_jit_set_domain (MonoDomain *domain)
+mono_jit_thread_detach (gpointer cookie, gpointer *dummy)
 {
-       if (domain)
-               mono_domain_set (domain, TRUE);
+       MonoDomain *domain, *orig;
+
+       if (!mono_threads_is_coop_enabled ()) {
+               orig = (MonoDomain*) cookie;
+
+               if (orig)
+                       mono_domain_set (orig, TRUE);
+       } else {
+               orig = (MonoDomain*) *dummy;
+
+               domain = mono_domain_get ();
+               g_assert (domain);
+
+               /* it won't do anything if cookie is NULL
+                * thread state RUNNING -> (RUNNING|BLOCKING) */
+               mono_threads_reset_blocking_end (cookie, dummy);
+
+               if (orig != domain) {
+                       if (!orig)
+                               mono_domain_unset ();
+                       else
+                               mono_domain_set (orig, TRUE);
+               }
+       }
 }
 
 /**
@@ -3777,8 +3849,8 @@ register_icalls (void)
        register_icall (mono_trace_enter_method, "mono_trace_enter_method", NULL, TRUE);
        register_icall (mono_trace_leave_method, "mono_trace_leave_method", NULL, TRUE);
        register_icall (mono_get_lmf_addr, "mono_get_lmf_addr", "ptr", TRUE);
-       register_icall (mono_jit_thread_attach, "mono_jit_thread_attach", "ptr ptr", TRUE);
-       register_icall (mono_jit_set_domain, "mono_jit_set_domain", "void ptr", TRUE);
+       register_icall (mono_jit_thread_attach, "mono_jit_thread_attach", "ptr ptr ptr", TRUE);
+       register_icall (mono_jit_thread_detach, "mono_jit_thread_detach", "void ptr ptr", TRUE);
        register_icall (mono_domain_get, "mono_domain_get", "ptr", TRUE);
 
        register_icall (mono_llvm_throw_exception, "mono_llvm_throw_exception", "void object", TRUE);
index f4d3666fa3127a71e0f96d1f4b90df52d0111c4c..5ebe8e3b1429305c31aef5f79670ec5f9a132982 100644 (file)
@@ -1605,8 +1605,9 @@ typedef struct {
        /* Points to a MonoGSharedVtMethodRuntimeInfo at runtime */
        MonoInst *gsharedvt_info_var;
 
-       /* For native-to-managed wrappers, the saved old domain */
-       MonoInst *orig_domain_var;
+       /* For native-to-managed wrappers, CEE_MONO_JIT_(AT|DE)TACH opcodes */
+       MonoInst *attach_cookie;
+       MonoInst *attach_dummy;
 
        MonoInst *lmf_var;
        MonoInst *lmf_addr_var;
@@ -2369,8 +2370,8 @@ MonoLMF * mono_get_lmf                      (void);
 MonoLMF** mono_get_lmf_addr                 (void);
 void      mono_set_lmf                      (MonoLMF *lmf);
 MonoJitTlsData* mono_get_jit_tls            (void);
-MONO_API MonoDomain *mono_jit_thread_attach          (MonoDomain *domain);
-MONO_API void      mono_jit_set_domain               (MonoDomain *domain);
+MONO_API gpointer  mono_jit_thread_attach            (MonoDomain *domain, gpointer *dummy);
+MONO_API void      mono_jit_thread_detach            (gpointer cookie, gpointer *dummy);
 gint32    mono_get_jit_tls_offset           (void);
 gint32    mono_get_lmf_tls_offset           (void);
 gint32    mono_get_lmf_addr_tls_offset      (void);
index e92d3720656aedef8edd71c808d08874b841e643..d11059161c5d43f493300169f88c47a9c7aa77c0 100644 (file)
@@ -486,7 +486,6 @@ mono_jit_init_version
 mono_jit_parse_options
 mono_jit_set_aot_mode
 mono_jit_set_aot_only
-mono_jit_set_domain
 mono_jit_set_trace_options
 mono_jit_thread_attach
 mono_ldstr
index dc32801d088054e5a333d5ebdaaf7fdbbfb700c8..aaa6969cfff791a071dde4a280cc1a519050d791 100644 (file)
@@ -488,7 +488,6 @@ mono_jit_init_version
 mono_jit_parse_options
 mono_jit_set_aot_mode
 mono_jit_set_aot_only
-mono_jit_set_domain
 mono_jit_set_trace_options
 mono_jit_thread_attach
 mono_ldstr