Skip to content

Commit 55f2bc6

Browse files
authored
[mini] Use atomics instead of loader lock for JIT wrappers and trampolines (#104038)
* [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 instead 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 520af01 commit 55f2bc6

File tree

3 files changed

+12
-18
lines changed

3 files changed

+12
-18
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,9 @@ typedef struct {
588588
// have both of them to be non-NULL.
589589
const char *name;
590590
gconstpointer func;
591+
// use CAS to write
591592
gconstpointer wrapper;
593+
// use CAS to write
592594
gconstpointer trampoline;
593595
MonoMethodSignature *sig;
594596
const char *c_symbol;

src/mono/mono/metadata/icall.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7205,6 +7205,8 @@ mono_create_icall_signatures (void)
72057205
}
72067206
}
72077207

7208+
/* LOCKING: does not take locks. does not use an atomic write to info->wrapper
7209+
*/
72087210
void
72097211
mono_register_jit_icall_info (MonoJitICallInfo *info, gconstpointer func, const char *name, MonoMethodSignature *sig, gboolean avoid_wrapper, const char *c_symbol)
72107212
{
@@ -7218,6 +7220,7 @@ mono_register_jit_icall_info (MonoJitICallInfo *info, gconstpointer func, const
72187220
// Fill in wrapper ahead of time, to just be func, to avoid
72197221
// later initializing it to anything else. So therefore, no wrapper.
72207222
if (avoid_wrapper) {
7223+
// not using CAS, because its idempotent
72217224
info->wrapper = func;
72227225
} else {
72237226
// Leave it alone in case of a race.

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

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -663,22 +663,19 @@ mono_icall_get_wrapper_full (MonoJitICallInfo* callinfo, gboolean do_compile)
663663
addr = mono_compile_method_checked (wrapper, error);
664664
mono_error_assert_ok (error);
665665
mono_memory_barrier ();
666-
callinfo->wrapper = addr;
667-
return addr;
666+
mono_atomic_cas_ptr ((volatile gpointer*)&callinfo->wrapper, (gpointer)addr, NULL);
667+
return (gconstpointer)mono_atomic_load_ptr((volatile gpointer*)&callinfo->wrapper);
668668
} else {
669669
if (callinfo->trampoline)
670670
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, (gpointer)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, p, NULL);
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)