From 12a15cdf4a7507167b8ef0deb0c67575fd723037 Mon Sep 17 00:00:00 2001 From: fanyang-mono Date: Tue, 25 Jun 2024 12:32:25 -0400 Subject: [PATCH 1/3] Fix the race condition issue of aot_module loading --- .../eventpipe/ep-rt-mono-runtime-provider.c | 6 +- src/mono/mono/mini/aot-runtime.c | 55 +++++++++++++------ src/mono/mono/mini/interp/transform.c | 3 +- src/mono/mono/mini/mini-amd64.c | 3 +- src/mono/mono/mini/mini.h | 3 + 5 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c b/src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c index ed00c37e1a1fa..379b7bde8a1ca 100644 --- a/src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c +++ b/src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c @@ -815,7 +815,7 @@ get_module_event_data ( module_data->module_flags = MODULE_FLAGS_MANIFEST_MODULE; if (image && image->dynamic) module_data->module_flags |= MODULE_FLAGS_DYNAMIC_MODULE; - if (image && image->aot_module) + if (image && image->aot_module && (image->aot_module != AOT_MODULE_NOT_FOUND)) module_data->module_flags |= MODULE_FLAGS_NATIVE_MODULE; module_data->module_il_path = NULL; @@ -905,7 +905,7 @@ fire_assembly_events ( if (assembly->dynamic) assembly_flags |= ASSEMBLY_FLAGS_DYNAMIC_ASSEMBLY; - if (assembly->image && assembly->image->aot_module) { + if (assembly->image && assembly->image->aot_module && (assembly->image->aot_module != AOT_MODULE_NOT_FOUND)) { assembly_flags |= ASSEMBLY_FLAGS_NATIVE_ASSEMBLY; } @@ -2152,7 +2152,7 @@ get_assembly_event_data ( if (assembly->dynamic) assembly_data->assembly_flags |= ASSEMBLY_FLAGS_DYNAMIC_ASSEMBLY; - if (assembly->image && assembly->image->aot_module) + if (assembly->image && assembly->image->aot_module && (assembly->image->aot_module != AOT_MODULE_NOT_FOUND)) assembly_data->assembly_flags |= ASSEMBLY_FLAGS_NATIVE_ASSEMBLY; assembly_data->assembly_name = mono_stringify_assembly_name (&assembly->aname); diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index b35d6b4f65d60..a995ebc12a1ad 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -1828,7 +1828,7 @@ init_amodule_got (MonoAotModule *amodule, gboolean preinit) if (mono_defaults.corlib) { MonoAotModule *mscorlib_amodule = mono_defaults.corlib->aot_module; - if (mscorlib_amodule) + if (mscorlib_amodule && (mscorlib_amodule != AOT_MODULE_NOT_FOUND)) amodule->shared_got [i] = mscorlib_amodule->got; } else { amodule->shared_got [i] = amodule->got; @@ -2143,7 +2143,7 @@ load_aot_module (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer mono_dl_close (sofile, close_error); mono_error_cleanup (close_error); } - assembly->image->aot_module = NULL; + assembly->image->aot_module = AOT_MODULE_NOT_FOUND; return; } @@ -2525,7 +2525,7 @@ load_container_amodule (MonoAssemblyLoadContext *alc) load_aot_module(alc, assm, NULL, error); mono_memory_barrier (); - g_assert (assm->image->aot_module); + g_assert (assm->image->aot_module && (assm->image->aot_module != AOT_MODULE_NOT_FOUND)); container_amodule = assm->image->aot_module; } @@ -2599,7 +2599,7 @@ mono_aot_get_method_from_vt_slot (MonoVTable *vtable, int slot, MonoError *error error_init (error); - if (MONO_CLASS_IS_INTERFACE_INTERNAL (klass) || m_class_get_rank (klass) || !amodule) + if (MONO_CLASS_IS_INTERFACE_INTERNAL (klass) || m_class_get_rank (klass) || !amodule || (amodule == AOT_MODULE_NOT_FOUND)) return NULL; info = &amodule->blob [mono_aot_get_offset (amodule->class_info_offsets, mono_metadata_token_index (m_class_get_type_token (klass)) - 1)]; @@ -2635,7 +2635,7 @@ mono_aot_get_cached_class_info (MonoClass *klass, MonoCachedClassInfo *res) guint8 *p; gboolean err; - if (m_class_get_rank (klass) || !m_class_get_type_token (klass) || !amodule) + if (m_class_get_rank (klass) || !m_class_get_type_token (klass) || !amodule || (amodule == AOT_MODULE_NOT_FOUND)) return FALSE; p = (guint8*)&amodule->blob [mono_aot_get_offset (amodule->class_info_offsets, mono_metadata_token_index (m_class_get_type_token (klass)) - 1)]; @@ -2675,7 +2675,7 @@ mono_aot_get_class_from_name (MonoImage *image, const char *name_space, const ch uint32_t debug_hash; #endif - if (!amodule || !amodule->class_name_table) + if (!amodule || (amodule == AOT_MODULE_NOT_FOUND) || !amodule->class_name_table) return FALSE; amodule_lock (amodule); @@ -2784,7 +2784,7 @@ mono_aot_get_weak_field_indexes (MonoImage *image) { MonoAotModule *amodule = image->aot_module; - if (!amodule) + if (!amodule || (amodule == AOT_MODULE_NOT_FOUND)) return NULL; #if ENABLE_WEAK_ATTR @@ -3478,7 +3478,7 @@ mono_aot_get_unwind_info (MonoJitInfo *ji, guint32 *unwind_info_len) amodule = ji->d.aot_info; else amodule = m_class_get_image (jinfo_get_method (ji)->klass)->aot_module; - g_assert (amodule); + g_assert (amodule && (amodule != AOT_MODULE_NOT_FOUND)); g_assert (ji->from_aot); if (!amodule_contains_code_addr (amodule, code)) { @@ -3615,7 +3615,7 @@ mono_aot_find_jit_info (MonoImage *image, gpointer addr) int methods_len; gboolean async; - if (!amodule) + if (!amodule || (amodule == AOT_MODULE_NOT_FOUND)) return NULL; addr = MINI_FTNPTR_TO_ADDR (addr); @@ -4510,7 +4510,7 @@ find_aot_method_in_amodule (MonoAotModule *code_amodule, MonoMethod *method, gui // the caching breaking. The solution seems to be to cache using the "metadata" amodule. MonoAotModule *metadata_amodule = m_class_get_image (method->klass)->aot_module; - if (!metadata_amodule || metadata_amodule->out_of_date || !code_amodule || code_amodule->out_of_date) + if (!metadata_amodule || (metadata_amodule == AOT_MODULE_NOT_FOUND) || metadata_amodule->out_of_date || !code_amodule || code_amodule->out_of_date) return 0xffffff; table = code_amodule->extra_method_table; @@ -4685,7 +4685,7 @@ find_aot_method (MonoMethod *method, MonoAotModule **out_amodule) /* Try the method's module first */ *out_amodule = m_class_get_image (method->klass)->aot_module; - index = find_aot_method_in_amodule (m_class_get_image (method->klass)->aot_module, method, hash); + index = find_aot_method_in_amodule (*out_amodule, method, hash); if (index != 0xffffff) return index; @@ -4920,14 +4920,35 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) MonoClass *klass = method->klass; MonoMethod *orig_method = method; guint32 method_index; - MonoAotModule *amodule = m_class_get_image (klass)->aot_module; + MonoImage *image = m_class_get_image (klass); guint8 *code = NULL; gboolean cache_result = FALSE; ERROR_DECL (inner_error); error_init (error); - if (!amodule) + if (!(image->aot_module)) { + // aot_module was uninitialized + MonoMethodHeader *header = mono_method_get_header_checked (method, error); + if (!header) { + return NULL; + } else if (header->code_size != 0) { + return NULL; + } else { + // IL code for the method body doesn't exist. Try waiting for aot_module to be loaded probably by another thread + int count = 0; + while (!(image->aot_module) && (count < 10)) { + g_usleep (100); + count++; + } + if (!(image->aot_module)) + return NULL; + } + } + + MonoAotModule *amodule = image->aot_module; + + if (amodule == AOT_MODULE_NOT_FOUND) return NULL; if (amodule->out_of_date) @@ -5181,7 +5202,7 @@ mono_aot_get_method_from_token (MonoImage *image, guint32 token, MonoError *erro error_init (error); - if (!aot_module) + if (!aot_module || aot_module == AOT_MODULE_NOT_FOUND) return NULL; method_index = mono_metadata_token_index (token) - 1; @@ -5683,7 +5704,7 @@ get_mscorlib_aot_module (void) MonoAotModule *amodule; image = mono_defaults.corlib; - if (image && image->aot_module) + if (image && image->aot_module && (image->aot_module != AOT_MODULE_NOT_FOUND)) amodule = image->aot_module; else amodule = mscorlib_aot_module; @@ -6135,7 +6156,7 @@ ui16_idx_comparer (const void *key, const void *member) static gboolean aot_is_slim_amodule (MonoAotModule *amodule) { - if (!amodule) + if (!amodule || amodule == AOT_MODULE_NOT_FOUND) return FALSE; /* "slim" only applies to mscorlib.dll */ @@ -6173,7 +6194,7 @@ mono_aot_get_unbox_trampoline (MonoMethod *method, gpointer addr) } else amodule = m_class_get_image (method->klass)->aot_module; - if (amodule == NULL || method_index == 0xffffff || aot_is_slim_amodule (amodule)) { + if (!amodule || amodule == AOT_MODULE_NOT_FOUND || method_index == 0xffffff || aot_is_slim_amodule (amodule)) { /* couldn't find unbox trampoline specifically generated for that * method. this should only happen when an unbox trampoline is needed * for `fullAOT code -> native-to-interp -> interp` transition if diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 49f97d62dee7d..7d6edc41e9283 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -1221,7 +1221,8 @@ mono_interp_jit_call_supported (MonoMethod *method, MonoMethodSignature *sig) if (!mono_jit_call_can_be_supported_by_interp (method, sig, mono_llvm_only)) return FALSE; - if (mono_aot_only && m_class_get_image (method->klass)->aot_module && !(method->iflags & METHOD_IMPL_ATTRIBUTE_SYNCHRONIZED)) { + MonoAotModule *amodule = m_class_get_image (method->klass)->aot_module; + if (mono_aot_only && amodule && (amodule != AOT_MODULE_NOT_FOUND) && !(method->iflags & METHOD_IMPL_ATTRIBUTE_SYNCHRONIZED)) { ERROR_DECL (error); mono_class_init_internal (method->klass); gpointer addr = mono_aot_get_method (method, error); diff --git a/src/mono/mono/mini/mini-amd64.c b/src/mono/mono/mini/mini-amd64.c index fa7e9c90b0148..76b42da79296f 100644 --- a/src/mono/mono/mini/mini-amd64.c +++ b/src/mono/mono/mini/mini-amd64.c @@ -3125,7 +3125,8 @@ emit_call (MonoCompile *cfg, MonoCallInst *call, guint8 *code, MonoJitICallId ji MonoMethod* const method = call->method; - if (m_class_get_image (method->klass)->aot_module) + MonoAotModule *amodule = m_class_get_image (method->klass)->aot_module; + if (amodule && (amodule != AOT_MODULE_NOT_FOUND)) /* The callee might be an AOT method */ near_call = FALSE; if (method->dynamic) diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index 215abfee0fb5f..97196e7e125cb 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -139,6 +139,9 @@ typedef struct SeqPointInfo SeqPointInfo; //XXX this ignores if t is byref #define MONO_TYPE_IS_PRIMITIVE_SCALAR(t) ((((((t)->type >= MONO_TYPE_BOOLEAN && (t)->type <= MONO_TYPE_U8) || ((t)->type >= MONO_TYPE_I && (t)->type <= MONO_TYPE_U))))) +// Used by MonoImage:aot_module to indicate aot_module was not found +#define AOT_MODULE_NOT_FOUND GINT_TO_POINTER (-1) + typedef struct { MonoClass *klass; From d9f7bf87082dcbd7bc0f0d35421e72f0b4af2083 Mon Sep 17 00:00:00 2001 From: fanyang-mono Date: Tue, 25 Jun 2024 18:28:04 -0400 Subject: [PATCH 2/3] Clean up error after get the header --- src/mono/mono/mini/aot-runtime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index a995ebc12a1ad..ade3c73c91751 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -4929,7 +4929,8 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) if (!(image->aot_module)) { // aot_module was uninitialized - MonoMethodHeader *header = mono_method_get_header_checked (method, error); + MonoMethodHeader *header = mono_method_get_header_checked (method, inner_error); + mono_error_cleanup (inner_error); if (!header) { return NULL; } else if (header->code_size != 0) { From e7283ba47ae5088dc276e50914afc1084962fb9e Mon Sep 17 00:00:00 2001 From: fanyang-mono Date: Wed, 26 Jun 2024 14:01:47 -0400 Subject: [PATCH 3/3] Address review feedback --- src/mono/mono/mini/aot-runtime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/mini/aot-runtime.c b/src/mono/mono/mini/aot-runtime.c index ade3c73c91751..2c7d6084373c4 100644 --- a/src/mono/mono/mini/aot-runtime.c +++ b/src/mono/mono/mini/aot-runtime.c @@ -2092,6 +2092,7 @@ load_aot_module (MonoAssemblyLoadContext *alc, MonoAssembly *assembly, gpointer g_free (aot_name); } #endif + assembly->image->aot_module = AOT_MODULE_NOT_FOUND; return; } } @@ -4938,7 +4939,7 @@ mono_aot_get_method (MonoMethod *method, MonoError *error) } else { // IL code for the method body doesn't exist. Try waiting for aot_module to be loaded probably by another thread int count = 0; - while (!(image->aot_module) && (count < 10)) { + while (!(image->aot_module) && (count < 10)) { // The threshold of count should never be removed to prevent deadlock. g_usleep (100); count++; }