Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mono] Fix the race condition issue of aot_module loading #103975

Merged
merged 4 commits into from
Jun 28, 2024
Merged

[mono] Fix the race condition issue of aot_module loading #103975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12a15cd
Fix the race condition issue of aot_module loading
fanyang-mono Jun 25, 2024
d9f7bf8
Clean up error after get the header
fanyang-mono Jun 25, 2024
e7283ba
Address review feedback
fanyang-mono Jun 26, 2024
cdc620c
Merge remote-tracking branch 'origin/main' into fix_race_condition
fanyang-mono Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/mono/mono/eventpipe/ep-rt-mono-runtime-provider.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
57 changes: 40 additions & 17 deletions src/mono/mono/mini/aot-runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -2143,7 +2144,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;
}

Expand Down Expand Up @@ -2525,7 +2526,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;
}

Expand Down Expand Up @@ -2599,7 +2600,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)];
Expand Down Expand Up @@ -2635,7 +2636,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)];
Expand Down Expand Up @@ -2675,7 +2676,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);
Expand Down Expand Up @@ -2784,7 +2785,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
Expand Down Expand Up @@ -3478,7 +3479,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)) {
Expand Down Expand Up @@ -3615,7 +3616,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);
Expand Down Expand Up @@ -4510,7 +4511,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;
Expand Down Expand Up @@ -4685,7 +4686,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;

Expand Down Expand Up @@ -4920,14 +4921,36 @@ 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, inner_error);
mono_error_cleanup (inner_error);
if (!header) {
return NULL;
} else if (header->code_size != 0) {
return NULL;
fanyang-mono marked this conversation as resolved.
Show resolved Hide resolved
} 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)) { // The threshold of count should never be removed to prevent deadlock.
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)
Expand Down Expand Up @@ -5181,7 +5204,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;
Expand Down Expand Up @@ -5683,7 +5706,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;
Expand Down Expand Up @@ -6135,7 +6158,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 */
Expand Down Expand Up @@ -6173,7 +6196,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
Expand Down
3 changes: 2 additions & 1 deletion src/mono/mono/mini/interp/transform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/mono/mono/mini/mini-amd64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions src/mono/mono/mini/mini.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading