From 18b8f513a0a6660cd667fd6825fa6d7dd7bf49f1 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Wed, 19 Aug 2020 19:47:26 -0500 Subject: [PATCH 1/4] [WASM] Add satellite assemblies bundle and hook to register them --- eng/testing/tests.mobile.targets | 1 + .../tests/SatelliteAssemblies.cs | 1 - src/mono/mono/metadata/assembly.c | 115 +++++++++++++++--- src/mono/mono/metadata/domain-internals.h | 3 +- src/mono/mono/metadata/domain.c | 4 +- src/mono/mono/metadata/image-internals.h | 3 + src/mono/mono/metadata/image.c | 42 ++++++- src/mono/mono/metadata/loaded-images.c | 5 +- src/mono/mono/metadata/loader-internals.h | 8 ++ .../mono/metadata/mono-private-unstable.h | 12 ++ src/mono/wasm/runtime/driver.c | 44 +++++++ src/mono/wasm/runtime/library_mono.js | 12 ++ .../WasmAppBuilder/WasmAppBuilder.cs | 30 ++++- 13 files changed, 252 insertions(+), 28 deletions(-) diff --git a/eng/testing/tests.mobile.targets b/eng/testing/tests.mobile.targets index 039b0d9a39b3c5..38a4080de10e36 100644 --- a/eng/testing/tests.mobile.targets +++ b/eng/testing/tests.mobile.targets @@ -147,6 +147,7 @@ MainAssembly="$(PublishDir)WasmTestRunner.dll" MainJS="$(MonoProjectRoot)\wasm\runtime-test.js" ExtraAssemblies="@(ExtraAssemblies)" + SatelliteAssemblies="@(WasmSatelliteAssemblies)" FilesToIncludeInFileSystem="@(WasmFilesToIncludeInFileSystem)" AssemblySearchPaths="@(AssemblySearchPaths)" /> diff --git a/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs b/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs index 7851469527bced..7473ed34a6143f 100644 --- a/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs +++ b/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs @@ -189,7 +189,6 @@ public static IEnumerable SatelliteLoadsCorrectly_TestData() [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInvariantGlobalization))] [MemberData(nameof(SatelliteLoadsCorrectly_TestData))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/39379", TestPlatforms.Browser)] public void SatelliteLoadsCorrectly(string alc, string assemblyName, string culture) { AssemblyName satelliteAssemblyName = new AssemblyName(assemblyName + ".resources"); diff --git a/src/mono/mono/metadata/assembly.c b/src/mono/mono/metadata/assembly.c index 1cf6d950ce3f25..60c55c08f7e7b6 100644 --- a/src/mono/mono/metadata/assembly.c +++ b/src/mono/mono/metadata/assembly.c @@ -375,6 +375,10 @@ mono_assemblies_unlock () /* If defined, points to the bundled assembly information */ static const MonoBundledAssembly **bundles; +#ifdef ENABLE_NETCORE +static const MonoBundledSatelliteAssembly **satellite_bundles; +#endif + static mono_mutex_t assembly_binding_mutex; /* Loaded assembly binding info */ @@ -1643,18 +1647,18 @@ load_reference_by_aname_individual_asmctx (MonoAssemblyName *aname, MonoAssembly } #else static MonoAssembly * -search_bundle_for_assembly (MonoAssemblyLoadContext *alc, MonoAssemblyName *aname) +search_bundle_for_assembly (MonoAssemblyLoadContext *alc, MonoAssemblyName *aname, gboolean is_satellite) { - if (bundles == NULL) + if ((bundles == NULL && !is_satellite) || (satellite_bundles == NULL && is_satellite)) return NULL; MonoImageOpenStatus status; MonoImage *image; MonoAssemblyLoadRequest req; - image = mono_assembly_open_from_bundle (alc, aname->name, &status, FALSE); + image = mono_assembly_open_from_bundle (alc, aname->name, &status, FALSE, aname->culture); if (!image) { char *name = g_strdup_printf ("%s.dll", aname->name); - image = mono_assembly_open_from_bundle (alc, name, &status, FALSE); + image = mono_assembly_open_from_bundle (alc, name, &status, FALSE, aname->culture); } if (image) { mono_assembly_request_prepare_load (&req, MONO_ASMCTX_DEFAULT, alc); @@ -1709,8 +1713,8 @@ netcore_load_reference (MonoAssemblyName *aname, MonoAssemblyLoadContext *alc, M goto leave; } - if (bundles != NULL) { - reference = search_bundle_for_assembly (alc, aname); + if (bundles != NULL || satellite_bundles != NULL) { + reference = search_bundle_for_assembly (alc, aname, is_satellite); if (reference) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Assembly found in the bundle: '%s'.", aname->name); goto leave; @@ -1769,6 +1773,50 @@ netcore_load_reference (MonoAssemblyName *aname, MonoAssemblyLoadContext *alc, M return reference; } +MonoImage * +netcore_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char* culture) +{ + int i; + char *name; + MonoImage *image = NULL; + gboolean is_satellite = culture && culture[0] != 0; + + if (!is_satellite) + { + if (!bundles) + return NULL; + + name = g_path_get_basename (filename); + for (i = 0; !image && bundles [i]; ++i) { + if (strcmp (bundles [i]->name, name) == 0) { + image = mono_image_open_from_data_internal (alc, (char*)bundles [i]->data, bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); + break; + } + } + } + else + { + if (!satellite_bundles) + return NULL; + + name = strdup(filename); + for (i = 0; !image && satellite_bundles [i]; ++i) { + if (strcmp (satellite_bundles [i]->culture, culture) == 0 && strcmp (satellite_bundles [i]->name, name) == 0) { + image = mono_image_open_from_data_internal (alc, (char*)satellite_bundles [i]->data, satellite_bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); + break; + } + } + } + if (image) { + mono_image_addref (image); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Assembly Loader loaded assembly from bundle: '%s'.", name); + g_free (name); + return image; + } + g_free (name); + return NULL; +} + #endif /* ENABLE_NETCORE */ /** @@ -2524,17 +2572,20 @@ absolute_dir (const gchar *filename) * returns NULL */ MonoImage * -mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly) +mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char* culture) { - int i; - char *name; - gchar *lowercase_filename; - MonoImage *image = NULL; - gboolean is_satellite = FALSE; +#ifdef ENABLE_NETCORE + return netcore_open_from_bundle (alc, filename, status, refonly, culture); +#else /* * we do a very simple search for bundled assemblies: it's not a general * purpose assembly loading mechanism. */ + int i; + char *name; + MonoImage *image = NULL; + gchar *lowercase_filename; + gboolean is_satellite = FALSE; if (!bundles) return NULL; @@ -2545,12 +2596,7 @@ mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filena name = g_path_get_basename (filename); for (i = 0; !image && bundles [i]; ++i) { if (strcmp (bundles [i]->name, is_satellite ? filename : name) == 0) { -#ifdef ENABLE_NETCORE - // Since bundled images don't exist on disk, don't give them a legit filename - image = mono_image_open_from_data_internal (alc, (char*)bundles [i]->data, bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); -#else image = mono_image_open_from_data_internal (alc, (char*)bundles [i]->data, bundles [i]->size, FALSE, status, refonly, FALSE, name, name); -#endif break; } } @@ -2562,6 +2608,8 @@ mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filena } g_free (name); return NULL; + +#endif /* ENABLE_NETCORE */ } /** @@ -2715,8 +2763,12 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest // If VM built with mkbundle loaded_from_bundle = FALSE; +#ifdef ENABLE_NETCORE + if (bundles != NULL || satellite_bundles != NULL) { +#else if (bundles != NULL) { - image = mono_assembly_open_from_bundle (load_req.alc, fname, status, refonly); +#endif + image = mono_assembly_open_from_bundle (load_req.alc, fname, status, refonly, NULL); loaded_from_bundle = image != NULL; } @@ -5298,6 +5350,33 @@ mono_register_bundled_assemblies (const MonoBundledAssembly **assemblies) bundles = assemblies; } +#ifdef ENABLE_NETCORE + +/** + * mono_create_new_bundled_satellite_assembly: + */ +MONO_API MonoBundledSatelliteAssembly * +mono_create_new_bundled_satellite_assembly (const char *name, const char *culture, const unsigned char *data, unsigned int size) +{ + MonoBundledSatelliteAssembly *satellite_assembly = g_new0 (MonoBundledSatelliteAssembly, 1); + satellite_assembly->name = strdup(name); + satellite_assembly->culture = strdup(culture); + satellite_assembly->data = data; + satellite_assembly->size = size; + return satellite_assembly; +} + +/** + * mono_register_bundled_satellite_assemblies: + */ +void +mono_register_bundled_satellite_assemblies (const MonoBundledSatelliteAssembly **assemblies) +{ + satellite_bundles = assemblies; +} + +#endif /* ENABLE_NETCORE */ + #define MONO_DECLSEC_FORMAT_10 0x3C #define MONO_DECLSEC_FORMAT_20 0x2E #define MONO_DECLSEC_FIELD 0x53 diff --git a/src/mono/mono/metadata/domain-internals.h b/src/mono/mono/metadata/domain-internals.h index 0985b671df7f52..4b79f0ceac1619 100644 --- a/src/mono/mono/metadata/domain-internals.h +++ b/src/mono/mono/metadata/domain-internals.h @@ -651,7 +651,8 @@ mono_domain_assembly_open_internal (MonoDomain *domain, MonoAssemblyLoadContext MonoImage *mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, - gboolean refonly); + gboolean refonly, + const char *culture); MonoAssembly * mono_try_assembly_resolve (MonoAssemblyLoadContext *alc, const char *fname, MonoAssembly *requesting, gboolean refonly, MonoError *error); diff --git a/src/mono/mono/metadata/domain.c b/src/mono/mono/metadata/domain.c index 4f4294adda2e3c..0faba3dcbe91cf 100644 --- a/src/mono/mono/metadata/domain.c +++ b/src/mono/mono/metadata/domain.c @@ -582,7 +582,7 @@ mono_init_internal (const char *filename, const char *exe_filename, const char * runtimes = get_runtimes_from_exe (exe_filename, &exe_image); #ifdef HOST_WIN32 if (!exe_image) { - exe_image = mono_assembly_open_from_bundle (mono_domain_default_alc (domain), exe_filename, NULL, FALSE); + exe_image = mono_assembly_open_from_bundle (mono_domain_default_alc (domain), exe_filename, NULL, FALSE, NULL); if (!exe_image) exe_image = mono_image_open (exe_filename, NULL); } @@ -1974,7 +1974,7 @@ get_runtimes_from_exe (const char *file, MonoImage **out_image) } /* Look for a runtime with the exact version */ - image = mono_assembly_open_from_bundle (mono_domain_default_alc (mono_domain_get ()), file, NULL, FALSE); + image = mono_assembly_open_from_bundle (mono_domain_default_alc (mono_domain_get ()), file, NULL, FALSE, NULL); if (image == NULL) image = mono_image_open (file, NULL); diff --git a/src/mono/mono/metadata/image-internals.h b/src/mono/mono/metadata/image-internals.h index 1ec204b335b006..22965a70548242 100644 --- a/src/mono/mono/metadata/image-internals.h +++ b/src/mono/mono/metadata/image-internals.h @@ -9,6 +9,9 @@ #include #include +char * +mono_image_get_name_with_culture_if_needed (MonoImage *image); + MonoImage* mono_image_loaded_internal (MonoAssemblyLoadContext *alc, const char *name, mono_bool refonly); diff --git a/src/mono/mono/metadata/image.c b/src/mono/mono/metadata/image.c index c82e2c516ed513..ad559c94ac98df 100644 --- a/src/mono/mono/metadata/image.c +++ b/src/mono/mono/metadata/image.c @@ -1846,34 +1846,70 @@ mono_image_loaded_by_guid (const char *guid) return mono_image_loaded_by_guid_internal (guid, FALSE); } +#ifdef ENABLE_NETCORE +static const char * +mono_get_image_culture (MonoImage *image) +{ + MonoTableInfo *t = &image->tables [MONO_TABLE_ASSEMBLY]; + if (!t->rows) + return NULL; + + guint32 cols [MONO_ASSEMBLY_SIZE]; + mono_metadata_decode_row (t, 0, cols, MONO_ASSEMBLY_SIZE); + return mono_metadata_string_heap (image, cols [MONO_ASSEMBLY_CULTURE]); +} +#endif + +char * +mono_image_get_name_with_culture_if_needed (MonoImage *image) +{ +#ifndef ENABLE_NETCORE + return g_strdup (image->name); +#else + if (!g_str_has_prefix (image->name, "data-") && + !g_path_is_absolute (image->name)) + { + const char *culture = mono_get_image_culture (image); + + if (culture && culture[0] != 0) + return g_strdup_printf ("(%s)%s", culture, image->name); + } + + return g_strdup (image->name); +#endif +} + static MonoImage * register_image (MonoLoadedImages *li, MonoImage *image, gboolean *problematic) { MonoImage *image2; + char *name = mono_image_get_name_with_culture_if_needed (image); GHashTable *loaded_images = mono_loaded_images_get_hash (li, image->ref_only); mono_images_lock (); - image2 = (MonoImage *)g_hash_table_lookup (loaded_images, image->name); + image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name); if (image2) { /* Somebody else beat us to it */ mono_image_addref (image2); mono_images_unlock (); mono_image_close (image); + g_free (name); return image2; } GHashTable *loaded_images_by_name = mono_loaded_images_get_by_name_hash (li, image->ref_only); - g_hash_table_insert (loaded_images, image->name, image); + g_hash_table_insert (loaded_images, name, image); if (image->assembly_name && (g_hash_table_lookup (loaded_images_by_name, image->assembly_name) == NULL)) g_hash_table_insert (loaded_images_by_name, (char *) image->assembly_name, image); mono_images_unlock (); if (mono_is_problematic_image (image)) { - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Registering %s, problematic image '%s'", image->ref_only ? "REFONLY" : "default", image->name); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Registering %s, problematic image '%s'", image->ref_only ? "REFONLY" : "default", name); if (problematic) *problematic = TRUE; } + g_free (name); return image; } diff --git a/src/mono/mono/metadata/loaded-images.c b/src/mono/mono/metadata/loaded-images.c index 91f3fd51c69ad8..d7357ed0d9c131 100644 --- a/src/mono/mono/metadata/loaded-images.c +++ b/src/mono/mono/metadata/loaded-images.c @@ -1,6 +1,7 @@ #include "config.h" #include "mono/metadata/loaded-images-internals.h" +#include "mono/metadata/image-internals.h" #include "mono/metadata/metadata-internals.h" #include "mono/utils/mono-logger-internals.h" @@ -100,7 +101,9 @@ mono_loaded_images_remove_image (MonoImage *image) loaded_images = mono_loaded_images_get_hash (li, image->ref_only); loaded_images_by_name = mono_loaded_images_get_by_name_hash (li, image->ref_only); - image2 = (MonoImage *)g_hash_table_lookup (loaded_images, image->name); + + char *name = mono_image_get_name_with_culture_if_needed (image); + image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name); if (image == image2) { /* This is not true if we are called from mono_image_open () */ g_hash_table_remove (loaded_images, image->name); diff --git a/src/mono/mono/metadata/loader-internals.h b/src/mono/mono/metadata/loader-internals.h index 43134540d59ab7..df08a1a0032539 100644 --- a/src/mono/mono/metadata/loader-internals.h +++ b/src/mono/mono/metadata/loader-internals.h @@ -60,6 +60,14 @@ struct _MonoAssemblyLoadContext { // Maps malloc-ed char* pinvoke scope -> MonoDl* GHashTable *pinvoke_scopes; }; + +struct _MonoBundledSatelliteAssembly { + const char *name; + const char *culture; + const unsigned char *data; + unsigned int size; +}; + #endif /* ENABLE_NETCORE */ void diff --git a/src/mono/mono/metadata/mono-private-unstable.h b/src/mono/mono/metadata/mono-private-unstable.h index c7d2531a7c744c..f92b0d0a8e7b17 100644 --- a/src/mono/mono/metadata/mono-private-unstable.h +++ b/src/mono/mono/metadata/mono-private-unstable.h @@ -27,4 +27,16 @@ mono_install_assembly_preload_hook_v3 (MonoAssemblyPreLoadFuncV3 func, void *use MONO_API MONO_RT_EXTERNAL_ONLY MonoAssemblyLoadContextGCHandle mono_alc_get_default_gchandle (void); +#ifdef ENABLE_NETCORE + +typedef struct _MonoBundledSatelliteAssembly MonoBundledSatelliteAssembly; + +MONO_API void +mono_register_bundled_satellite_assemblies (const MonoBundledSatelliteAssembly **assemblies); + +MONO_API MonoBundledSatelliteAssembly * +mono_create_new_bundled_satellite_assembly (const char *name, const char *culture, const unsigned char *data, unsigned int size); + +#endif /* ENABLE_NETCORE */ + #endif /*__MONO_METADATA_MONO_PRIVATE_UNSTABLE_H__*/ diff --git a/src/mono/wasm/runtime/driver.c b/src/mono/wasm/runtime/driver.c index 264a5b9d7eb643..2fbf7b7b4a569f 100644 --- a/src/mono/wasm/runtime/driver.c +++ b/src/mono/wasm/runtime/driver.c @@ -15,6 +15,11 @@ #include // FIXME: unavailable in emscripten // #include + +#ifdef ENABLE_NETCORE +#include +#endif + #include #include #include @@ -196,6 +201,29 @@ mono_wasm_add_assembly (const char *name, const unsigned char *data, unsigned in return mono_has_pdb_checksum (data, size); } +#ifdef ENABLE_NETCORE + +typedef struct WasmSatelliteAssembly_ WasmSatelliteAssembly; + +struct WasmSatelliteAssembly_ { + MonoBundledSatelliteAssembly *assembly; + WasmSatelliteAssembly *next; +}; + +static WasmSatelliteAssembly *satellite_assemblies; +static int satellite_assembly_count; + +EMSCRIPTEN_KEEPALIVE void +mono_wasm_add_satellite_assembly (const char *name, const char *culture, const unsigned char *data, unsigned int size) +{ + WasmSatelliteAssembly *entry = g_new0 (WasmSatelliteAssembly, 1); + entry->assembly = mono_create_new_bundled_satellite_assembly(name, culture, data, size); + entry->next = satellite_assemblies; + satellite_assemblies = entry; + ++satellite_assembly_count; +} +#endif // ENABLE_NETCORE + EMSCRIPTEN_KEEPALIVE void mono_wasm_setenv (const char *name, const char *value) { @@ -470,6 +498,22 @@ mono_wasm_load_runtime (const char *unused, int debug_level) mono_register_bundled_assemblies ((const MonoBundledAssembly **)bundle_array); } +#ifdef ENABLE_NETCORE + + if (satellite_assembly_count) { + MonoBundledSatelliteAssembly **satellite_bundle_array = g_new0 (MonoBundledSatelliteAssembly*, satellite_assembly_count + 1); + WasmSatelliteAssembly *cur = satellite_assemblies; + int i = 0; + while (cur) { + satellite_bundle_array [i] = cur->assembly; + cur = cur->next; + ++i; + } + mono_register_bundled_satellite_assemblies ((const MonoBundledSatelliteAssembly **)satellite_bundle_array); + } + +#endif // ENABLE_NETCORE + mono_trace_init (); mono_trace_set_log_handler (wasm_logger, NULL); root_domain = mono_jit_init_version ("mono", "v4.0.30319"); diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 909de6790777b1..1d1d3af007f801 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -1077,6 +1077,7 @@ var MonoSupportLib = { var offset = null; switch (asset.behavior) { + case "resource": case "assembly": ctx.loaded_files.push ({ url: url, file: virtualName}); case "heap": @@ -1136,6 +1137,10 @@ var MonoSupportLib = { else console.error ("Error loading ICU asset", asset.name); } + else if(asset.behavior === "resource") + { + ctx.mono_wasm_add_satellite_assembly (virtualName, asset.culture, offset, bytes.length); + } }, // deprecated @@ -1179,6 +1184,7 @@ var MonoSupportLib = { // behavior: (required) determines how the asset will be handled once loaded: // "heap": store asset into the native heap // "assembly": load asset as a managed assembly (or debugging information) + // "resource": load asset as a managed resource assembly // "icu": load asset as an ICU data archive // "vfs": load asset into the virtual filesystem (for fopen, File.Open, etc) // load_remote: (optional) if true, an attempt will be made to load the asset @@ -1289,6 +1295,7 @@ var MonoSupportLib = { tracing: args.diagnostic_tracing || false, pending_count: args.assets.length, mono_wasm_add_assembly: Module.cwrap ('mono_wasm_add_assembly', 'number', ['string', 'number', 'number']), + mono_wasm_add_satellite_assembly: Module.cwrap ('mono_wasm_add_satellite_assembly', 'void', ['string', 'string', 'number', 'number']), loaded_assets: Object.create (null), // dlls and pdbs, used by blazor and the debugger loaded_files: [], @@ -1381,6 +1388,11 @@ var MonoSupportLib = { if (sourcePrefix.trim() === "") { if (asset.behavior === "assembly") attemptUrl = locateFile (args.assembly_root + "/" + asset.name); + else if (asset.behavior === "resource") + { + var path = asset.culture !== '' ? `${asset.culture}/${asset.name}` : asset.name; + attemptUrl = locateFile (args.assembly_root + "/" + path); + } else attemptUrl = asset.name; } else { diff --git a/tools-local/tasks/mobile.tasks/WasmAppBuilder/WasmAppBuilder.cs b/tools-local/tasks/mobile.tasks/WasmAppBuilder/WasmAppBuilder.cs index 3fa63daf9ed67b..c011a324a0dd86 100644 --- a/tools-local/tasks/mobile.tasks/WasmAppBuilder/WasmAppBuilder.cs +++ b/tools-local/tasks/mobile.tasks/WasmAppBuilder/WasmAppBuilder.cs @@ -36,6 +36,7 @@ public class WasmAppBuilder : Task public ITaskItem[]? AssemblySearchPaths { get; set; } public int DebugLevel { get; set; } public ITaskItem[]? ExtraAssemblies { get; set; } + public ITaskItem[]? SatelliteAssemblies { get; set; } public ITaskItem[]? FilesToIncludeInFileSystem { get; set; } public ITaskItem[]? RemoteSources { get; set; } public bool InvariantGlobalization { get; set; } @@ -72,6 +73,17 @@ private class AssemblyEntry : AssetEntry public AssemblyEntry(string name) : base(name, "assembly") {} } + private class SatelliteAssemblyEntry : AssetEntry + { + public SatelliteAssemblyEntry(string name, string culture) : base(name, "resource") + { + CultureName = culture; + } + + [JsonPropertyName("culture")] + public string CultureName { get; set; } + } + private class VfsEntry : AssetEntry { public VfsEntry(string name) : base(name, "vfs") {} [JsonPropertyName("virtual_path")] @@ -147,17 +159,31 @@ public override bool Execute () File.WriteAllText(Path.Join(AppDir, "index.html"), html); foreach (var assembly in _assemblies.Values) { - config.Assets.Add(new AssemblyEntry (Path.GetFileName(assembly.Location))); + config.Assets.Add(new AssemblyEntry(Path.GetFileName(assembly.Location))); if (DebugLevel > 0) { var pdb = assembly.Location; pdb = Path.ChangeExtension(pdb, ".pdb"); if (File.Exists(pdb)) - config.Assets.Add(new AssemblyEntry (Path.GetFileName(pdb))); + config.Assets.Add(new AssemblyEntry(Path.GetFileName(pdb))); } } config.DebugLevel = DebugLevel; + if (SatelliteAssemblies != null) + { + foreach (var assembly in SatelliteAssemblies) + { + string culture = assembly.GetMetadata("CultureName") ?? string.Empty; + string fullPath = assembly.GetMetadata("Identity"); + string name = Path.GetFileName(fullPath); + string directory = Path.Join(AppDir, config.AssemblyRoot, culture); + Directory.CreateDirectory(directory); + File.Copy(fullPath, Path.Join(directory, name), true); + config.Assets.Add(new SatelliteAssemblyEntry(name, culture)); + } + } + if (FilesToIncludeInFileSystem != null) { string supportFilesDir = Path.Join(AppDir, "supportFiles"); From 77a73bffb6f99a321a2fe96c1d6ce32d2e76b848 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Wed, 26 Aug 2020 01:52:29 -0500 Subject: [PATCH 2/4] Fix test failures --- src/mono/mono/metadata/image.c | 16 ++++++++-------- src/mono/mono/metadata/loaded-images.c | 7 ++++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/mono/mono/metadata/image.c b/src/mono/mono/metadata/image.c index ad559c94ac98df..4928f5eee55034 100644 --- a/src/mono/mono/metadata/image.c +++ b/src/mono/mono/metadata/image.c @@ -1864,7 +1864,7 @@ char * mono_image_get_name_with_culture_if_needed (MonoImage *image) { #ifndef ENABLE_NETCORE - return g_strdup (image->name); + return NULL; #else if (!g_str_has_prefix (image->name, "data-") && !g_path_is_absolute (image->name)) @@ -1875,7 +1875,7 @@ mono_image_get_name_with_culture_if_needed (MonoImage *image) return g_strdup_printf ("(%s)%s", culture, image->name); } - return g_strdup (image->name); + return NULL; #endif } @@ -1883,33 +1883,33 @@ static MonoImage * register_image (MonoLoadedImages *li, MonoImage *image, gboolean *problematic) { MonoImage *image2; - char *name = mono_image_get_name_with_culture_if_needed (image); + char *name_with_culture = mono_image_get_name_with_culture_if_needed (image); GHashTable *loaded_images = mono_loaded_images_get_hash (li, image->ref_only); mono_images_lock (); - image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name); + image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name_with_culture != NULL ? name_with_culture : image->name); if (image2) { /* Somebody else beat us to it */ mono_image_addref (image2); mono_images_unlock (); mono_image_close (image); - g_free (name); + g_free (name_with_culture); return image2; } GHashTable *loaded_images_by_name = mono_loaded_images_get_by_name_hash (li, image->ref_only); - g_hash_table_insert (loaded_images, name, image); + g_hash_table_insert (loaded_images, name_with_culture != NULL ? name_with_culture : image->name, image); if (image->assembly_name && (g_hash_table_lookup (loaded_images_by_name, image->assembly_name) == NULL)) g_hash_table_insert (loaded_images_by_name, (char *) image->assembly_name, image); mono_images_unlock (); if (mono_is_problematic_image (image)) { - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Registering %s, problematic image '%s'", image->ref_only ? "REFONLY" : "default", name); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Registering %s, problematic image '%s'", image->ref_only ? "REFONLY" : "default", name_with_culture != NULL ? name_with_culture : image->name); if (problematic) *problematic = TRUE; } - g_free (name); + g_free (name_with_culture); return image; } diff --git a/src/mono/mono/metadata/loaded-images.c b/src/mono/mono/metadata/loaded-images.c index d7357ed0d9c131..2c034ae05686e9 100644 --- a/src/mono/mono/metadata/loaded-images.c +++ b/src/mono/mono/metadata/loaded-images.c @@ -102,16 +102,17 @@ mono_loaded_images_remove_image (MonoImage *image) loaded_images = mono_loaded_images_get_hash (li, image->ref_only); loaded_images_by_name = mono_loaded_images_get_by_name_hash (li, image->ref_only); - char *name = mono_image_get_name_with_culture_if_needed (image); - image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name); + char *name_with_culture = mono_image_get_name_with_culture_if_needed (image); + image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name_with_culture != NULL ? name_with_culture : image->name); if (image == image2) { /* This is not true if we are called from mono_image_open () */ - g_hash_table_remove (loaded_images, image->name); + g_hash_table_remove (loaded_images, name_with_culture != NULL ? name_with_culture : image->name); } if (image->assembly_name && (g_hash_table_lookup (loaded_images_by_name, image->assembly_name) == image)) g_hash_table_remove (loaded_images_by_name, (char *) image->assembly_name); proceed = TRUE; + g_free (name_with_culture); done: mono_images_unlock (); From 2462047fad5b6c06c81a65a78cc0b63913855e53 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Thu, 27 Aug 2020 15:45:56 -0500 Subject: [PATCH 3/4] PR Feedback --- src/mono/mono/metadata/assembly.c | 131 ++++++++---------- src/mono/mono/metadata/image.c | 30 ++-- src/mono/mono/metadata/loaded-images.c | 12 +- src/mono/mono/metadata/loader-internals.h | 14 +- .../mono/metadata/mono-private-unstable.h | 4 - src/mono/wasm/runtime/driver.c | 11 +- src/mono/wasm/runtime/library_mono.js | 6 +- 7 files changed, 96 insertions(+), 112 deletions(-) diff --git a/src/mono/mono/metadata/assembly.c b/src/mono/mono/metadata/assembly.c index 60c55c08f7e7b6..f83fb2363e7881 100644 --- a/src/mono/mono/metadata/assembly.c +++ b/src/mono/mono/metadata/assembly.c @@ -375,9 +375,7 @@ mono_assemblies_unlock () /* If defined, points to the bundled assembly information */ static const MonoBundledAssembly **bundles; -#ifdef ENABLE_NETCORE static const MonoBundledSatelliteAssembly **satellite_bundles; -#endif static mono_mutex_t assembly_binding_mutex; @@ -1773,48 +1771,24 @@ netcore_load_reference (MonoAssemblyName *aname, MonoAssemblyLoadContext *alc, M return reference; } -MonoImage * -netcore_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char* culture) +static MonoImage * +netcore_open_from_satellite_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char *culture) { + if (!satellite_bundles) + return NULL; + int i; - char *name; MonoImage *image = NULL; - gboolean is_satellite = culture && culture[0] != 0; - - if (!is_satellite) - { - if (!bundles) - return NULL; - - name = g_path_get_basename (filename); - for (i = 0; !image && bundles [i]; ++i) { - if (strcmp (bundles [i]->name, name) == 0) { - image = mono_image_open_from_data_internal (alc, (char*)bundles [i]->data, bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); - break; - } - } - } - else - { - if (!satellite_bundles) - return NULL; - - name = strdup(filename); - for (i = 0; !image && satellite_bundles [i]; ++i) { - if (strcmp (satellite_bundles [i]->culture, culture) == 0 && strcmp (satellite_bundles [i]->name, name) == 0) { - image = mono_image_open_from_data_internal (alc, (char*)satellite_bundles [i]->data, satellite_bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); - break; - } + char *name = g_strdup (filename); + for (i = 0; !image && satellite_bundles [i]; ++i) { + if (strcmp (satellite_bundles [i]->name, name) == 0 && strcmp (satellite_bundles [i]->culture, culture) == 0) { + image = mono_image_open_from_data_internal (alc, (char *)satellite_bundles [i]->data, satellite_bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); + break; } } - if (image) { - mono_image_addref (image); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Assembly Loader loaded assembly from bundle: '%s'.", name); - g_free (name); - return image; - } + g_free (name); - return NULL; + return image; } #endif /* ENABLE_NETCORE */ @@ -2562,6 +2536,31 @@ absolute_dir (const gchar *filename) return res; } +static MonoImage * +open_from_bundle_internal (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, gboolean is_satellite) +{ + if (!bundles) + return NULL; + + int i; + MonoImage *image = NULL; + char *name = is_satellite ? g_strdup(filename) : g_path_get_basename (filename); + for (i = 0; !image && bundles [i]; ++i) { + if (strcmp (bundles [i]->name, name) == 0) { +#ifdef ENABLE_NETCORE + // Since bundled images don't exist on disk, don't give them a legit filename + image = mono_image_open_from_data_internal (alc, (char*)bundles [i]->data, bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); +#else + image = mono_image_open_from_data_internal (alc, (char*)bundles [i]->data, bundles [i]->size, FALSE, status, refonly, FALSE, name, name); +#endif + break; + } + } + + g_free (name); + return image; +} + /** * mono_assembly_open_from_bundle: * \param filename Filename requested @@ -2572,44 +2571,36 @@ absolute_dir (const gchar *filename) * returns NULL */ MonoImage * -mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char* culture) +mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char *culture) { -#ifdef ENABLE_NETCORE - return netcore_open_from_bundle (alc, filename, status, refonly, culture); -#else /* * we do a very simple search for bundled assemblies: it's not a general * purpose assembly loading mechanism. */ - int i; - char *name; MonoImage *image = NULL; - gchar *lowercase_filename; - gboolean is_satellite = FALSE; - +#ifndef ENABLE_NETCORE if (!bundles) return NULL; - lowercase_filename = g_utf8_strdown (filename, -1); - is_satellite = g_str_has_suffix (lowercase_filename, ".resources.dll"); + gchar *lowercase_filename = g_utf8_strdown (filename, -1); + gboolean is_satellite = g_str_has_suffix (lowercase_filename, ".resources.dll"); g_free (lowercase_filename); - name = g_path_get_basename (filename); - for (i = 0; !image && bundles [i]; ++i) { - if (strcmp (bundles [i]->name, is_satellite ? filename : name) == 0) { - image = mono_image_open_from_data_internal (alc, (char*)bundles [i]->data, bundles [i]->size, FALSE, status, refonly, FALSE, name, name); - break; - } + image = open_from_bundle_internal (alc, filename, status, refonly, is_satellite); +#else + gboolean is_satellite = culture && culture [0] != 0;; + if (is_satellite) + { + image = netcore_open_from_satellite_bundle (alc, filename, status, refonly, culture); + } else { + image = open_from_bundle_internal (alc, filename, status, refonly, FALSE); } +#endif + if (image) { mono_image_addref (image); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Assembly Loader loaded assembly from bundle: '%s'.", is_satellite ? filename : name); - g_free (name); - return image; + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Assembly Loader loaded assembly from bundle: '%s'.", filename); } - g_free (name); - return NULL; - -#endif /* ENABLE_NETCORE */ + return image; } /** @@ -2763,11 +2754,7 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest // If VM built with mkbundle loaded_from_bundle = FALSE; -#ifdef ENABLE_NETCORE if (bundles != NULL || satellite_bundles != NULL) { -#else - if (bundles != NULL) { -#endif image = mono_assembly_open_from_bundle (load_req.alc, fname, status, refonly, NULL); loaded_from_bundle = image != NULL; } @@ -5350,17 +5337,15 @@ mono_register_bundled_assemblies (const MonoBundledAssembly **assemblies) bundles = assemblies; } -#ifdef ENABLE_NETCORE - /** * mono_create_new_bundled_satellite_assembly: */ -MONO_API MonoBundledSatelliteAssembly * +MonoBundledSatelliteAssembly * mono_create_new_bundled_satellite_assembly (const char *name, const char *culture, const unsigned char *data, unsigned int size) { MonoBundledSatelliteAssembly *satellite_assembly = g_new0 (MonoBundledSatelliteAssembly, 1); - satellite_assembly->name = strdup(name); - satellite_assembly->culture = strdup(culture); + satellite_assembly->name = strdup (name); + satellite_assembly->culture = strdup (culture); satellite_assembly->data = data; satellite_assembly->size = size; return satellite_assembly; @@ -5372,11 +5357,11 @@ mono_create_new_bundled_satellite_assembly (const char *name, const char *cultur void mono_register_bundled_satellite_assemblies (const MonoBundledSatelliteAssembly **assemblies) { +#ifdef ENABLE_NETCORE satellite_bundles = assemblies; +#endif } -#endif /* ENABLE_NETCORE */ - #define MONO_DECLSEC_FORMAT_10 0x3C #define MONO_DECLSEC_FORMAT_20 0x2E #define MONO_DECLSEC_FIELD 0x53 diff --git a/src/mono/mono/metadata/image.c b/src/mono/mono/metadata/image.c index 4928f5eee55034..32804b339116b4 100644 --- a/src/mono/mono/metadata/image.c +++ b/src/mono/mono/metadata/image.c @@ -1846,9 +1846,8 @@ mono_image_loaded_by_guid (const char *guid) return mono_image_loaded_by_guid_internal (guid, FALSE); } -#ifdef ENABLE_NETCORE static const char * -mono_get_image_culture (MonoImage *image) +get_image_culture (MonoImage *image) { MonoTableInfo *t = &image->tables [MONO_TABLE_ASSEMBLY]; if (!t->rows) @@ -1858,58 +1857,63 @@ mono_get_image_culture (MonoImage *image) mono_metadata_decode_row (t, 0, cols, MONO_ASSEMBLY_SIZE); return mono_metadata_string_heap (image, cols [MONO_ASSEMBLY_CULTURE]); } -#endif char * mono_image_get_name_with_culture_if_needed (MonoImage *image) { -#ifndef ENABLE_NETCORE - return NULL; -#else if (!g_str_has_prefix (image->name, "data-") && !g_path_is_absolute (image->name)) { - const char *culture = mono_get_image_culture (image); + const char *culture = get_image_culture (image); - if (culture && culture[0] != 0) - return g_strdup_printf ("(%s)%s", culture, image->name); + if (culture && culture [0] != 0) + return g_strdup_printf ("%s/%s", culture, image->name); } return NULL; -#endif } static MonoImage * register_image (MonoLoadedImages *li, MonoImage *image, gboolean *problematic) { MonoImage *image2; + char *name = image->name; +#ifdef ENABLE_NETCORE char *name_with_culture = mono_image_get_name_with_culture_if_needed (image); + if (name_with_culture) { + name = name_with_culture; + } +#endif GHashTable *loaded_images = mono_loaded_images_get_hash (li, image->ref_only); mono_images_lock (); - image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name_with_culture != NULL ? name_with_culture : image->name); + image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name); if (image2) { /* Somebody else beat us to it */ mono_image_addref (image2); mono_images_unlock (); mono_image_close (image); +#ifdef ENABLE_NETCORE g_free (name_with_culture); +#endif return image2; } GHashTable *loaded_images_by_name = mono_loaded_images_get_by_name_hash (li, image->ref_only); - g_hash_table_insert (loaded_images, name_with_culture != NULL ? name_with_culture : image->name, image); + g_hash_table_insert (loaded_images, name, image); if (image->assembly_name && (g_hash_table_lookup (loaded_images_by_name, image->assembly_name) == NULL)) g_hash_table_insert (loaded_images_by_name, (char *) image->assembly_name, image); mono_images_unlock (); if (mono_is_problematic_image (image)) { - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Registering %s, problematic image '%s'", image->ref_only ? "REFONLY" : "default", name_with_culture != NULL ? name_with_culture : image->name); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Registering %s, problematic image '%s'", image->ref_only ? "REFONLY" : "default", name); if (problematic) *problematic = TRUE; } +#ifdef ENABLE_NETCORE g_free (name_with_culture); +#endif return image; } diff --git a/src/mono/mono/metadata/loaded-images.c b/src/mono/mono/metadata/loaded-images.c index 2c034ae05686e9..e28906a6dc3de8 100644 --- a/src/mono/mono/metadata/loaded-images.c +++ b/src/mono/mono/metadata/loaded-images.c @@ -102,17 +102,25 @@ mono_loaded_images_remove_image (MonoImage *image) loaded_images = mono_loaded_images_get_hash (li, image->ref_only); loaded_images_by_name = mono_loaded_images_get_by_name_hash (li, image->ref_only); + char *name = image->name; +#ifdef ENABLE_NETCORE char *name_with_culture = mono_image_get_name_with_culture_if_needed (image); - image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name_with_culture != NULL ? name_with_culture : image->name); + if (name_with_culture) { + name = name_with_culture; + } +#endif + image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name); if (image == image2) { /* This is not true if we are called from mono_image_open () */ - g_hash_table_remove (loaded_images, name_with_culture != NULL ? name_with_culture : image->name); + g_hash_table_remove (loaded_images, name); } if (image->assembly_name && (g_hash_table_lookup (loaded_images_by_name, image->assembly_name) == image)) g_hash_table_remove (loaded_images_by_name, (char *) image->assembly_name); proceed = TRUE; +#ifdef ENABLE_NETCORE g_free (name_with_culture); +#endif done: mono_images_unlock (); diff --git a/src/mono/mono/metadata/loader-internals.h b/src/mono/mono/metadata/loader-internals.h index df08a1a0032539..97bb3508780cd2 100644 --- a/src/mono/mono/metadata/loader-internals.h +++ b/src/mono/mono/metadata/loader-internals.h @@ -26,6 +26,13 @@ typedef struct _MonoLoadedImages MonoLoadedImages; typedef struct _MonoAssemblyLoadContext MonoAssemblyLoadContext; +struct _MonoBundledSatelliteAssembly { + const char *name; + const char *culture; + const unsigned char *data; + unsigned int size; +}; + #ifndef DISABLE_DLLMAP typedef struct _MonoDllMap MonoDllMap; struct _MonoDllMap { @@ -61,13 +68,6 @@ struct _MonoAssemblyLoadContext { GHashTable *pinvoke_scopes; }; -struct _MonoBundledSatelliteAssembly { - const char *name; - const char *culture; - const unsigned char *data; - unsigned int size; -}; - #endif /* ENABLE_NETCORE */ void diff --git a/src/mono/mono/metadata/mono-private-unstable.h b/src/mono/mono/metadata/mono-private-unstable.h index f92b0d0a8e7b17..562e9f1c4df635 100644 --- a/src/mono/mono/metadata/mono-private-unstable.h +++ b/src/mono/mono/metadata/mono-private-unstable.h @@ -27,8 +27,6 @@ mono_install_assembly_preload_hook_v3 (MonoAssemblyPreLoadFuncV3 func, void *use MONO_API MONO_RT_EXTERNAL_ONLY MonoAssemblyLoadContextGCHandle mono_alc_get_default_gchandle (void); -#ifdef ENABLE_NETCORE - typedef struct _MonoBundledSatelliteAssembly MonoBundledSatelliteAssembly; MONO_API void @@ -37,6 +35,4 @@ mono_register_bundled_satellite_assemblies (const MonoBundledSatelliteAssembly * MONO_API MonoBundledSatelliteAssembly * mono_create_new_bundled_satellite_assembly (const char *name, const char *culture, const unsigned char *data, unsigned int size); -#endif /* ENABLE_NETCORE */ - #endif /*__MONO_METADATA_MONO_PRIVATE_UNSTABLE_H__*/ diff --git a/src/mono/wasm/runtime/driver.c b/src/mono/wasm/runtime/driver.c index 2fbf7b7b4a569f..05f1486e58d268 100644 --- a/src/mono/wasm/runtime/driver.c +++ b/src/mono/wasm/runtime/driver.c @@ -201,8 +201,6 @@ mono_wasm_add_assembly (const char *name, const unsigned char *data, unsigned in return mono_has_pdb_checksum (data, size); } -#ifdef ENABLE_NETCORE - typedef struct WasmSatelliteAssembly_ WasmSatelliteAssembly; struct WasmSatelliteAssembly_ { @@ -217,12 +215,11 @@ EMSCRIPTEN_KEEPALIVE void mono_wasm_add_satellite_assembly (const char *name, const char *culture, const unsigned char *data, unsigned int size) { WasmSatelliteAssembly *entry = g_new0 (WasmSatelliteAssembly, 1); - entry->assembly = mono_create_new_bundled_satellite_assembly(name, culture, data, size); + entry->assembly = mono_create_new_bundled_satellite_assembly (name, culture, data, size); entry->next = satellite_assemblies; satellite_assemblies = entry; ++satellite_assembly_count; } -#endif // ENABLE_NETCORE EMSCRIPTEN_KEEPALIVE void mono_wasm_setenv (const char *name, const char *value) @@ -498,10 +495,8 @@ mono_wasm_load_runtime (const char *unused, int debug_level) mono_register_bundled_assemblies ((const MonoBundledAssembly **)bundle_array); } -#ifdef ENABLE_NETCORE - if (satellite_assembly_count) { - MonoBundledSatelliteAssembly **satellite_bundle_array = g_new0 (MonoBundledSatelliteAssembly*, satellite_assembly_count + 1); + MonoBundledSatelliteAssembly **satellite_bundle_array = g_new0 (MonoBundledSatelliteAssembly *, satellite_assembly_count + 1); WasmSatelliteAssembly *cur = satellite_assemblies; int i = 0; while (cur) { @@ -512,8 +507,6 @@ mono_wasm_load_runtime (const char *unused, int debug_level) mono_register_bundled_satellite_assemblies ((const MonoBundledSatelliteAssembly **)satellite_bundle_array); } -#endif // ENABLE_NETCORE - mono_trace_init (); mono_trace_set_log_handler (wasm_logger, NULL); root_domain = mono_jit_init_version ("mono", "v4.0.30319"); diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 1d1d3af007f801..d7325c2aa5070a 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -1137,8 +1137,7 @@ var MonoSupportLib = { else console.error ("Error loading ICU asset", asset.name); } - else if(asset.behavior === "resource") - { + else if (asset.behavior === "resource") { ctx.mono_wasm_add_satellite_assembly (virtualName, asset.culture, offset, bytes.length); } }, @@ -1388,8 +1387,7 @@ var MonoSupportLib = { if (sourcePrefix.trim() === "") { if (asset.behavior === "assembly") attemptUrl = locateFile (args.assembly_root + "/" + asset.name); - else if (asset.behavior === "resource") - { + else if (asset.behavior === "resource") { var path = asset.culture !== '' ? `${asset.culture}/${asset.name}` : asset.name; attemptUrl = locateFile (args.assembly_root + "/" + path); } From bc92c478f0aa37b8e447341fc932c338439f582a Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Thu, 3 Sep 2020 11:32:24 -0500 Subject: [PATCH 4/4] PR Feedback and add test to make sure load from path respects culture --- .../tests/SatelliteAssemblies.cs | 21 ++++++++++++++++++- src/mono/mono/metadata/assembly.c | 13 ++++++------ src/mono/mono/metadata/image.c | 10 +++++++-- src/mono/mono/metadata/loaded-images.c | 6 +++--- src/mono/mono/metadata/loader-internals.h | 1 - src/mono/wasm/runtime/driver.c | 3 +-- 6 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs b/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs index 7473ed34a6143f..c8d74fb0177397 100644 --- a/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs +++ b/src/libraries/System.Runtime.Loader/tests/SatelliteAssemblies.cs @@ -189,7 +189,7 @@ public static IEnumerable SatelliteLoadsCorrectly_TestData() [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInvariantGlobalization))] [MemberData(nameof(SatelliteLoadsCorrectly_TestData))] - public void SatelliteLoadsCorrectly(string alc, string assemblyName, string culture) + public void SatelliteLoadsCorrectly_FromName(string alc, string assemblyName, string culture) { AssemblyName satelliteAssemblyName = new AssemblyName(assemblyName + ".resources"); satelliteAssemblyName.CultureInfo = new CultureInfo(culture); @@ -205,5 +205,24 @@ public void SatelliteLoadsCorrectly(string alc, string assemblyName, string cult Assert.Equal(AssemblyLoadContext.GetLoadContext(parentAssembly), AssemblyLoadContext.GetLoadContext(satelliteAssembly)); } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInvariantGlobalization))] + [MemberData(nameof(SatelliteLoadsCorrectly_TestData))] + public void SatelliteLoadsCorrectly_FromPath(string alc, string assemblyName, string culture) + { + string satelliteAssemblyName = assemblyName + ".resources.dll"; + + AssemblyLoadContext assemblyLoadContext = contexts[alc]; + + string assemblyPath = Path.Join(AppDomain.CurrentDomain.BaseDirectory, culture, satelliteAssemblyName); + Assembly satelliteAssembly = assemblyLoadContext.LoadFromAssemblyPath(assemblyPath); + + Assert.NotNull(satelliteAssembly); + + AssemblyName parentAssemblyName = new AssemblyName(assemblyName); + Assembly parentAssembly = assemblyLoadContext.LoadFromAssemblyName(parentAssemblyName); + + Assert.Equal(culture, satelliteAssembly.GetName().CultureName); + } } } diff --git a/src/mono/mono/metadata/assembly.c b/src/mono/mono/metadata/assembly.c index f83fb2363e7881..40fe00b9ad11fc 100644 --- a/src/mono/mono/metadata/assembly.c +++ b/src/mono/mono/metadata/assembly.c @@ -1772,15 +1772,14 @@ netcore_load_reference (MonoAssemblyName *aname, MonoAssemblyLoadContext *alc, M } static MonoImage * -netcore_open_from_satellite_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char *culture) +open_from_satellite_bundle (MonoAssemblyLoadContext *alc, const char *filename, MonoImageOpenStatus *status, gboolean refonly, const char *culture) { if (!satellite_bundles) return NULL; - int i; MonoImage *image = NULL; char *name = g_strdup (filename); - for (i = 0; !image && satellite_bundles [i]; ++i) { + for (int i = 0; !image && satellite_bundles [i]; ++i) { if (strcmp (satellite_bundles [i]->name, name) == 0 && strcmp (satellite_bundles [i]->culture, culture) == 0) { image = mono_image_open_from_data_internal (alc, (char *)satellite_bundles [i]->data, satellite_bundles [i]->size, FALSE, status, refonly, FALSE, name, NULL); break; @@ -2542,10 +2541,9 @@ open_from_bundle_internal (MonoAssemblyLoadContext *alc, const char *filename, M if (!bundles) return NULL; - int i; MonoImage *image = NULL; - char *name = is_satellite ? g_strdup(filename) : g_path_get_basename (filename); - for (i = 0; !image && bundles [i]; ++i) { + char *name = is_satellite ? g_strdup (filename) : g_path_get_basename (filename); + for (int i = 0; !image && bundles [i]; ++i) { if (strcmp (bundles [i]->name, name) == 0) { #ifdef ENABLE_NETCORE // Since bundled images don't exist on disk, don't give them a legit filename @@ -2590,7 +2588,7 @@ mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filena gboolean is_satellite = culture && culture [0] != 0;; if (is_satellite) { - image = netcore_open_from_satellite_bundle (alc, filename, status, refonly, culture); + image = open_from_satellite_bundle (alc, filename, status, refonly, culture); } else { image = open_from_bundle_internal (alc, filename, status, refonly, FALSE); } @@ -2755,6 +2753,7 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest // If VM built with mkbundle loaded_from_bundle = FALSE; if (bundles != NULL || satellite_bundles != NULL) { + /* We don't know the culture of the filename we're loading here, so this call is not culture aware. */ image = mono_assembly_open_from_bundle (load_req.alc, fname, status, refonly, NULL); loaded_from_bundle = image != NULL; } diff --git a/src/mono/mono/metadata/image.c b/src/mono/mono/metadata/image.c index 32804b339116b4..2d06947676e90a 100644 --- a/src/mono/mono/metadata/image.c +++ b/src/mono/mono/metadata/image.c @@ -1879,10 +1879,11 @@ register_image (MonoLoadedImages *li, MonoImage *image, gboolean *problematic) MonoImage *image2; char *name = image->name; #ifdef ENABLE_NETCORE + /* Since we register cultures by file name, we need to make this culture aware for + satellite assemblies */ char *name_with_culture = mono_image_get_name_with_culture_if_needed (image); - if (name_with_culture) { + if (name_with_culture) name = name_with_culture; - } #endif GHashTable *loaded_images = mono_loaded_images_get_hash (li, image->ref_only); @@ -2069,6 +2070,11 @@ mono_image_open_full (const char *fname, MonoImageOpenStatus *status, gboolean r return mono_image_open_a_lot (alc, fname, status, refonly, FALSE); } +/** + * mono_image_open_a_lot_parameterized + * this API is not culture aware, so if we load a satellite assembly for one culture by name + * via this API, and then try to load it with another culture we will return the first one. + */ static MonoImage * mono_image_open_a_lot_parameterized (MonoLoadedImages *li, MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status, gboolean refonly, gboolean load_from_context, gboolean *problematic) { diff --git a/src/mono/mono/metadata/loaded-images.c b/src/mono/mono/metadata/loaded-images.c index e28906a6dc3de8..c0fa0d5e6aa3ac 100644 --- a/src/mono/mono/metadata/loaded-images.c +++ b/src/mono/mono/metadata/loaded-images.c @@ -79,6 +79,7 @@ loaded_images_get_owner (MonoImage *image) gboolean mono_loaded_images_remove_image (MonoImage *image) { + char *name = NULL; gboolean proceed = FALSE; /* * Atomically decrement the refcount and remove ourselves from the hash tables, so @@ -102,12 +103,11 @@ mono_loaded_images_remove_image (MonoImage *image) loaded_images = mono_loaded_images_get_hash (li, image->ref_only); loaded_images_by_name = mono_loaded_images_get_by_name_hash (li, image->ref_only); - char *name = image->name; + name = image->name; #ifdef ENABLE_NETCORE char *name_with_culture = mono_image_get_name_with_culture_if_needed (image); - if (name_with_culture) { + if (name_with_culture) name = name_with_culture; - } #endif image2 = (MonoImage *)g_hash_table_lookup (loaded_images, name); if (image == image2) { diff --git a/src/mono/mono/metadata/loader-internals.h b/src/mono/mono/metadata/loader-internals.h index 97bb3508780cd2..b8ee8c8cd1cd91 100644 --- a/src/mono/mono/metadata/loader-internals.h +++ b/src/mono/mono/metadata/loader-internals.h @@ -67,7 +67,6 @@ struct _MonoAssemblyLoadContext { // Maps malloc-ed char* pinvoke scope -> MonoDl* GHashTable *pinvoke_scopes; }; - #endif /* ENABLE_NETCORE */ void diff --git a/src/mono/wasm/runtime/driver.c b/src/mono/wasm/runtime/driver.c index 05f1486e58d268..60368b99b060cf 100644 --- a/src/mono/wasm/runtime/driver.c +++ b/src/mono/wasm/runtime/driver.c @@ -16,9 +16,7 @@ // FIXME: unavailable in emscripten // #include -#ifdef ENABLE_NETCORE #include -#endif #include #include @@ -495,6 +493,7 @@ mono_wasm_load_runtime (const char *unused, int debug_level) mono_register_bundled_assemblies ((const MonoBundledAssembly **)bundle_array); } + /* In legacy satellite_assembly_count is always false */ if (satellite_assembly_count) { MonoBundledSatelliteAssembly **satellite_bundle_array = g_new0 (MonoBundledSatelliteAssembly *, satellite_assembly_count + 1); WasmSatelliteAssembly *cur = satellite_assemblies;