From 720c9f42c7a1d5b11b30c19232ab0b9b58cb10c1 Mon Sep 17 00:00:00 2001 From: Rodrigo Kumpera Date: Tue, 26 Nov 2013 15:10:44 -0500 Subject: [PATCH] [runtime] Introduce the icall lock to be used when looking up icall related information. --- data/lock-decoder/LockTracerDecoder.cs | 10 +++-- mono/metadata/icall.c | 58 ++++++++++++++++---------- mono/metadata/lock-tracer.h | 10 +++++ mono/mini/aot-compiler.c | 2 +- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/data/lock-decoder/LockTracerDecoder.cs b/data/lock-decoder/LockTracerDecoder.cs index b5a3f755ea5..f786e6b81dc 100644 --- a/data/lock-decoder/LockTracerDecoder.cs +++ b/data/lock-decoder/LockTracerDecoder.cs @@ -102,14 +102,15 @@ Global locks: Adding global locks is not to be taken lightly. The current lock hierarchy: -loader lock - domain lock - domain jit lock +loader lock (global) + domain lock (complex) + domain jit lock (complex) simple locks Examples: You can take the loader lock without holding a domain lock. - You cannot take a domain lock if the loader lock is held. + You can take the domain load while holding the loader lock + You cannot take the loader lock if only the domain lock is held. You cannot take a domain lock while holding the lock to another domain. */ @@ -120,6 +121,7 @@ public enum Lock { DomainLock, DomainAssembliesLock, DomainJitCodeHashLock, + IcallLock, } public class SimLock diff --git a/mono/metadata/icall.c b/mono/metadata/icall.c index 26871d5311a..450d7cd21e6 100644 --- a/mono/metadata/icall.c +++ b/mono/metadata/icall.c @@ -85,6 +85,7 @@ #include #include #include +#include #if defined (HOST_WIN32) #include @@ -7857,6 +7858,7 @@ icall_symbols [] = { #endif /* DISABLE_ICALL_TABLES */ +static mono_mutex_t icall_mutex; static GHashTable *icall_hash = NULL; static GHashTable *jit_icall_hash_name = NULL; static GHashTable *jit_icall_hash_addr = NULL; @@ -7894,6 +7896,19 @@ mono_icall_init (void) #endif icall_hash = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); + mono_mutex_init (&icall_mutex); +} + +static void +mono_icall_lock (void) +{ + mono_locks_mutex_acquire (&icall_mutex, IcallLock); +} + +static void +mono_icall_unlock (void) +{ + mono_locks_mutex_release (&icall_mutex, IcallLock); } void @@ -7902,16 +7917,17 @@ mono_icall_cleanup (void) g_hash_table_destroy (icall_hash); g_hash_table_destroy (jit_icall_hash_name); g_hash_table_destroy (jit_icall_hash_addr); + mono_mutex_destroy (&icall_mutex); } void mono_add_internal_call (const char *name, gconstpointer method) { - mono_loader_lock (); + mono_icall_lock (); g_hash_table_insert (icall_hash, g_strdup (name), (gpointer) method); - mono_loader_unlock (); + mono_icall_unlock (); } #ifndef DISABLE_ICALL_TABLES @@ -8074,23 +8090,23 @@ mono_lookup_internal_call (MonoMethod *method) sigstart [siglen + 2] = 0; g_free (tmpsig); - mono_loader_lock (); + mono_icall_lock (); res = g_hash_table_lookup (icall_hash, mname); if (res) { - mono_loader_unlock (); + mono_icall_unlock ();; return res; } /* try without signature */ *sigstart = 0; res = g_hash_table_lookup (icall_hash, mname); if (res) { - mono_loader_unlock (); + mono_icall_unlock (); return res; } #ifdef DISABLE_ICALL_TABLES - mono_loader_unlock (); + mono_icall_unlock (); /* Fail only when the result is actually used */ /* mono_marshal_get_native_wrapper () depends on this */ if (method->klass == mono_defaults.string_class && !strcmp (method->name, ".ctor")) @@ -8100,19 +8116,19 @@ mono_lookup_internal_call (MonoMethod *method) #else /* it wasn't found in the static call tables */ if (!imap) { - mono_loader_unlock (); + mono_icall_unlock (); return NULL; } res = find_method_icall (imap, sigstart - mlen); if (res) { - mono_loader_unlock (); + mono_icall_unlock (); return res; } /* try _with_ signature */ *sigstart = '('; res = find_method_icall (imap, sigstart - mlen); if (res) { - mono_loader_unlock (); + mono_icall_unlock (); return res; } @@ -8124,7 +8140,7 @@ mono_lookup_internal_call (MonoMethod *method) g_print ("If you see other errors or faults after this message they are probably related\n"); g_print ("and you need to fix your mono install first.\n"); - mono_loader_unlock (); + mono_icall_unlock (); return NULL; #endif @@ -8304,9 +8320,9 @@ mono_find_jit_icall_by_name (const char *name) MonoJitICallInfo *info; g_assert (jit_icall_hash_name); - mono_loader_lock (); + mono_icall_lock (); info = g_hash_table_lookup (jit_icall_hash_name, name); - mono_loader_unlock (); + mono_icall_unlock (); return info; } @@ -8316,9 +8332,9 @@ mono_find_jit_icall_by_addr (gconstpointer addr) MonoJitICallInfo *info; g_assert (jit_icall_hash_addr); - mono_loader_lock (); + mono_icall_lock (); info = g_hash_table_lookup (jit_icall_hash_addr, (gpointer)addr); - mono_loader_unlock (); + mono_icall_unlock (); return info; } @@ -8327,7 +8343,7 @@ mono_find_jit_icall_by_addr (gconstpointer addr) * mono_get_jit_icall_info: * * Return the hashtable mapping JIT icall names to MonoJitICallInfo structures. The - * caller should access it while holding the loader lock. + * caller should access it while holding the icall lock. */ GHashTable* mono_get_jit_icall_info (void) @@ -8346,20 +8362,20 @@ mono_lookup_jit_icall_symbol (const char *name) MonoJitICallInfo *info; const char *res = NULL; - mono_loader_lock (); + mono_icall_lock (); info = g_hash_table_lookup (jit_icall_hash_name, name); if (info) res = info->c_symbol; - mono_loader_unlock (); + mono_icall_unlock (); return res; } void mono_register_jit_icall_wrapper (MonoJitICallInfo *info, gconstpointer wrapper) { - mono_loader_lock (); + mono_icall_lock (); g_hash_table_insert (jit_icall_hash_addr, (gpointer)wrapper, info); - mono_loader_unlock (); + mono_icall_unlock (); } MonoJitICallInfo * @@ -8370,7 +8386,7 @@ mono_register_jit_icall_full (gconstpointer func, const char *name, MonoMethodSi g_assert (func); g_assert (name); - mono_loader_lock (); + mono_icall_lock (); if (!jit_icall_hash_name) { jit_icall_hash_name = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_free); @@ -8398,7 +8414,7 @@ mono_register_jit_icall_full (gconstpointer func, const char *name, MonoMethodSi g_hash_table_insert (jit_icall_hash_name, (gpointer)info->name, info); g_hash_table_insert (jit_icall_hash_addr, (gpointer)func, info); - mono_loader_unlock (); + mono_icall_unlock (); return info; } diff --git a/mono/metadata/lock-tracer.h b/mono/metadata/lock-tracer.h index 40beac748e8..ac9599ecbab 100644 --- a/mono/metadata/lock-tracer.h +++ b/mono/metadata/lock-tracer.h @@ -15,6 +15,7 @@ typedef enum { DomainLock, DomainAssembliesLock, DomainJitCodeHashLock, + IcallLock } RuntimeLocks; #ifdef LOCK_TRACER @@ -43,6 +44,15 @@ void mono_locks_lock_released (RuntimeLocks kind, gpointer lock) MONO_INTERNAL; LeaveCriticalSection (LOCK); \ } while (0) +#define mono_locks_mutex_acquire(LOCK, NAME) do { \ + mono_mutex_lock (LOCK); \ + mono_locks_lock_acquired (NAME, LOCK); \ +} while (0) + +#define mono_locks_mutex_release(LOCK, NAME) do { \ + mono_locks_lock_released (NAME, LOCK); \ + mono_mutex_unlock (LOCK); \ +} while (0) G_END_DECLS #endif /* __MONO_METADATA_LOCK_TRACER_H__ */ diff --git a/mono/mini/aot-compiler.c b/mono/mini/aot-compiler.c index 9723c34b4cd..f8de62212bc 100755 --- a/mono/mini/aot-compiler.c +++ b/mono/mini/aot-compiler.c @@ -3408,7 +3408,7 @@ add_wrappers (MonoAotCompile *acfg) #endif /* JIT icall wrappers */ - /* FIXME: locking */ + /* FIXME: locking - this is "safe" as full-AOT threads don't mutate the icall hash*/ g_hash_table_foreach (mono_get_jit_icall_info (), add_jit_icall_wrapper, acfg); } -- 2.25.1