Skip to content

Commit 7f6152f

Browse files
committed
[mini] Use atomics, instead of loader lock, for JIT wrappers
Related to #93686 While this doesn't eliminate all deadlocks related to the global loader lock and managed locks, it removes one unneeded use of the loader lock. The wrapper (and trampoline) of a JIT icall are only ever set from NULL to non-NULL. We can use atomics to deal with races instad of double checked locking. This was not the case historically, because the JIT info was dynamically allocated - so we used the loader lock to protect the integrity of the hash table
1 parent ff3c531 commit 7f6152f

File tree

5 files changed

+19
-28
lines changed

5 files changed

+19
-28
lines changed

src/mono/mono/metadata/class-internals.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -587,8 +587,8 @@ typedef struct {
587587
// have both of them to be non-NULL.
588588
const char *name;
589589
gconstpointer func;
590-
gconstpointer wrapper;
591-
gconstpointer trampoline;
590+
gconstpointer wrapper__;
591+
gconstpointer trampoline__;
592592
MonoMethodSignature *sig;
593593
const char *c_symbol;
594594
MonoMethod *wrapper_method;

src/mono/mono/metadata/icall.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7243,6 +7243,8 @@ mono_create_icall_signatures (void)
72437243
}
72447244
}
72457245

7246+
/* LOCKING: does not take locks. does not use an atomic write to info->wrapper
7247+
*/
72467248
void
72477249
mono_register_jit_icall_info (MonoJitICallInfo *info, gconstpointer func, const char *name, MonoMethodSignature *sig, gboolean avoid_wrapper, const char *c_symbol)
72487250
{
@@ -7256,7 +7258,7 @@ mono_register_jit_icall_info (MonoJitICallInfo *info, gconstpointer func, const
72567258
// Fill in wrapper ahead of time, to just be func, to avoid
72577259
// later initializing it to anything else. So therefore, no wrapper.
72587260
if (avoid_wrapper) {
7259-
info->wrapper = func;
7261+
info->wrapper__ = func;
72607262
} else {
72617263
// Leave it alone in case of a race.
72627264
}

src/mono/mono/mini/aot-compiler.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6942,7 +6942,7 @@ emit_and_reloc_code (MonoAotCompile *acfg, MonoMethod *method, guint8 *code, gui
69426942
}
69436943
} else if (patch_info->type == MONO_PATCH_INFO_JIT_ICALL_ID) {
69446944
MonoJitICallInfo * const info = mono_find_jit_icall_info (patch_info->data.jit_icall_id);
6945-
if (!got_only && info->func == info->wrapper) {
6945+
if (!got_only && info->func == info->wrapper__) {
69466946
const char * sym = NULL;
69476947
if (patch_info->data.jit_icall_id == MONO_JIT_ICALL_mono_dummy_runtime_init_callback) {
69486948
g_assert (acfg->aot_opts.static_link && acfg->aot_opts.runtime_init_callback != NULL);
@@ -10583,7 +10583,7 @@ mono_aot_get_direct_call_symbol (MonoJumpInfoType type, gconstpointer data)
1058310583
sym = lookup_direct_pinvoke_symbol_name_aot (llvm_acfg, method);
1058410584
} else if (type == MONO_PATCH_INFO_JIT_ICALL_ID && (direct_calls || (MonoJitICallId)(gsize)data == MONO_JIT_ICALL_mono_dummy_runtime_init_callback)) {
1058510585
MonoJitICallInfo const * const info = mono_find_jit_icall_info ((MonoJitICallId)(gsize)data);
10586-
if (info->func == info->wrapper) {
10586+
if (info->func == info->wrapper__) {
1058710587
if ((MonoJitICallId)(gsize)data == MONO_JIT_ICALL_mono_dummy_runtime_init_callback) {
1058810588
sym = llvm_acfg->aot_opts.runtime_init_callback;
1058910589
} else {

src/mono/mono/mini/mini-llvm.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2168,7 +2168,7 @@ get_callee_llvmonly (EmitContext *ctx, LLVMTypeRef llvm_sig, MonoJumpInfoType ty
21682168
if (type == MONO_PATCH_INFO_JIT_ICALL_ID) {
21692169
MonoJitICallInfo * const info = mono_find_jit_icall_info ((MonoJitICallId)(gsize)data);
21702170
g_assert (info);
2171-
if (info->func != info->wrapper) {
2171+
if (info->func != info->wrapper__) {
21722172
type = MONO_PATCH_INFO_METHOD;
21732173
data = mono_icall_get_wrapper_method (info);
21742174
callee_name = mono_aot_get_mangled_method_name ((MonoMethod*)data);
@@ -2211,7 +2211,7 @@ get_callee_llvmonly (EmitContext *ctx, LLVMTypeRef llvm_sig, MonoJumpInfoType ty
22112211
if (ctx->module->assembly->image == mono_get_corlib () && type == MONO_PATCH_INFO_JIT_ICALL_ID) {
22122212
MonoJitICallInfo * const info = mono_find_jit_icall_info ((MonoJitICallId)(gsize)data);
22132213

2214-
if (info->func != info->wrapper) {
2214+
if (info->func != info->wrapper__) {
22152215
type = MONO_PATCH_INFO_METHOD;
22162216
data = mono_icall_get_wrapper_method (info);
22172217
}

src/mono/mono/mini/mini-runtime.c

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -654,31 +654,28 @@ mono_icall_get_wrapper_full (MonoJitICallInfo* callinfo, gboolean do_compile)
654654
MonoMethod *wrapper;
655655
gconstpointer addr, trampoline;
656656

657-
if (callinfo->wrapper)
658-
return callinfo->wrapper;
657+
if (callinfo->wrapper__)
658+
return callinfo->wrapper__;
659659

660660
wrapper = mono_icall_get_wrapper_method (callinfo);
661661

662662
if (do_compile) {
663663
addr = mono_compile_method_checked (wrapper, error);
664664
mono_error_assert_ok (error);
665665
mono_memory_barrier ();
666-
callinfo->wrapper = addr;
666+
callinfo->wrapper__ = addr;
667667
return addr;
668668
} else {
669-
if (callinfo->trampoline)
670-
return callinfo->trampoline;
669+
if (callinfo->trampoline__)
670+
return callinfo->trampoline__;
671671
trampoline = mono_create_jit_trampoline (wrapper, error);
672672
mono_error_assert_ok (error);
673673
trampoline = mono_create_ftnptr ((gpointer)trampoline);
674674

675-
mono_loader_lock ();
676-
if (!callinfo->trampoline) {
677-
callinfo->trampoline = trampoline;
678-
}
679-
mono_loader_unlock ();
675+
676+
mono_atomic_cas_ptr ((volatile gpointer*)&callinfo->trampoline__, (void*)trampoline, NULL);
680677

681-
return callinfo->trampoline;
678+
return (gconstpointer)mono_atomic_load_ptr ((volatile gpointer*)&callinfo->trampoline__);
682679
}
683680
}
684681

@@ -2856,18 +2853,10 @@ mono_jit_compile_method_with_opt (MonoMethod *method, guint32 opt, gboolean jit_
28562853
p = mono_create_ftnptr (code);
28572854

28582855
if (callinfo) {
2859-
// FIXME Locking here is somewhat historical due to mono_register_jit_icall_wrapper taking loader lock.
2860-
// atomic_compare_exchange should suffice.
2861-
mono_loader_lock ();
2862-
mono_jit_lock ();
2863-
if (!callinfo->wrapper) {
2864-
callinfo->wrapper = p;
2865-
}
2866-
mono_jit_unlock ();
2867-
mono_loader_unlock ();
2856+
mono_atomic_cas_ptr ((volatile void*)&callinfo->wrapper__, NULL, p);
2857+
p = mono_atomic_load_ptr((volatile gpointer*)&callinfo->wrapper__);
28682858
}
28692859

2870-
// FIXME p or callinfo->wrapper or does not matter?
28712860
return p;
28722861
}
28732862

0 commit comments

Comments
 (0)