From 143c471eff107cb1b28152d5d36f89f1b21a95ee Mon Sep 17 00:00:00 2001 From: Ignacio Etcheverry Date: Wed, 22 Aug 2018 00:33:23 +0200 Subject: [PATCH] Mono: Fix weird crash when loading corlib --- modules/mono/mono_gd/gd_mono_assembly.cpp | 57 +++++++++++++++++------ modules/mono/mono_gd/gd_mono_assembly.h | 1 + 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/modules/mono/mono_gd/gd_mono_assembly.cpp b/modules/mono/mono_gd/gd_mono_assembly.cpp index 57797598e14..27ce39b6d76 100644 --- a/modules/mono/mono_gd/gd_mono_assembly.cpp +++ b/modules/mono/mono_gd/gd_mono_assembly.cpp @@ -42,9 +42,15 @@ #include "gd_mono_class.h" bool GDMonoAssembly::no_search = false; +bool GDMonoAssembly::in_preload = false; + Vector GDMonoAssembly::search_dirs; void GDMonoAssembly::assembly_load_hook(MonoAssembly *assembly, void *user_data) { + + if (no_search) + return; + // If our search and preload hooks fail to load the assembly themselves, the mono runtime still might. // Just do Assembly.LoadFrom("/Full/Path/On/Disk.dll"); // In this case, we wouldn't have the assembly known in GDMono, which causes crashes @@ -122,6 +128,8 @@ MonoAssembly *GDMonoAssembly::_search_hook(MonoAssemblyName *aname, void *user_d return res ? res->get_assembly() : NULL; } +static _THREAD_LOCAL_(MonoImage *) image_corlib_loading = NULL; + MonoAssembly *GDMonoAssembly::_preload_hook(MonoAssemblyName *aname, char **assemblies_path, void *user_data, bool refonly) { (void)user_data; // UNUSED @@ -149,10 +157,27 @@ MonoAssembly *GDMonoAssembly::_preload_hook(MonoAssemblyName *aname, char **asse } } + { + // If we find the assembly here, we load it with `mono_assembly_load_from_full`, + // which in turn invokes load hooks before returning the MonoAssembly to us. + // One of the load hooks is `load_aot_module`. This hook can end up calling preload hooks + // again for the same assembly in certain in certain circumstances (the `do_load_image` part). + // If this is the case and we return NULL due to the no_search condition below, + // it will result in an internal crash later on. Therefore we need to return the assembly we didn't + // get yet from `mono_assembly_load_from_full`. Luckily we have the image, which already got it. + // This must be done here. If done in search hooks, it would cause `mono_assembly_load_from_full` + // to think another MonoAssembly for this assembly was already loaded, making it delete its own, + // when in fact both pointers were the same... This hooks thing is confusing. + if (image_corlib_loading) { + return mono_image_get_assembly(image_corlib_loading); + } + } + if (no_search) return NULL; no_search = true; + in_preload = true; String name = mono_assembly_name_get_name(aname); bool has_extension = name.ends_with(".dll"); @@ -187,6 +212,7 @@ MonoAssembly *GDMonoAssembly::_preload_hook(MonoAssemblyName *aname, char **asse } no_search = false; + in_preload = false; return res ? res->get_assembly() : NULL; } @@ -209,23 +235,18 @@ GDMonoAssembly *GDMonoAssembly::_load_assembly_from(const String &p_name, const } void GDMonoAssembly::_wrap_mono_assembly(MonoAssembly *assembly) { - String p_name = mono_assembly_name_get_name(mono_assembly_get_name(assembly)); - GDMonoAssembly **existingassembly = GDMono::get_singleton()->get_loaded_assembly(p_name); - - if (no_search || existingassembly != NULL) { - // Not sure whether the existingassembly check matters - // since no_search handles it in most cases? - // Can't hurt. - return; - } + String name = mono_assembly_name_get_name(mono_assembly_get_name(assembly)); MonoImage *image = mono_assembly_get_image(assembly); - mono_image_addref(image); - GDMonoAssembly *gdassembly = memnew(GDMonoAssembly(p_name, mono_image_get_filename(image))); - gdassembly->assembly = assembly; - gdassembly->loaded = true; - gdassembly->image = image; + GDMonoAssembly *gdassembly = memnew(GDMonoAssembly(name, mono_image_get_filename(image))); + Error err = gdassembly->wrapper_for_image(image); + + if (err != OK) { + memdelete(gdassembly); + ERR_FAIL(); + } + MonoDomain *domain = mono_domain_get(); GDMono::get_singleton()->add_assembly(domain ? mono_domain_get_id(domain) : 0, gdassembly); } @@ -280,8 +301,16 @@ no_pdb: #endif + bool is_corlib_preload = in_preload && name == "mscorlib"; + + if (is_corlib_preload) + image_corlib_loading = image; + assembly = mono_assembly_load_from_full(image, image_filename.utf8().get_data(), &status, refonly); + if (is_corlib_preload) + image_corlib_loading = NULL; + ERR_FAIL_COND_V(status != MONO_IMAGE_OK || assembly == NULL, ERR_FILE_CANT_OPEN); loaded = true; diff --git a/modules/mono/mono_gd/gd_mono_assembly.h b/modules/mono/mono_gd/gd_mono_assembly.h index e8f2be62f8d..2c6d367fc6a 100644 --- a/modules/mono/mono_gd/gd_mono_assembly.h +++ b/modules/mono/mono_gd/gd_mono_assembly.h @@ -89,6 +89,7 @@ class GDMonoAssembly { #endif static bool no_search; + static bool in_preload; static Vector search_dirs; static void assembly_load_hook(MonoAssembly *assembly, void *user_data);