Skip to content

Commit

Permalink
[mono] Add test for wasm loader regression, fix loading from bundle
Browse files Browse the repository at this point in the history
Fixes dotnet/runtime#42207

The new test is based on the sample from that issue.

The naive fix (moving all bundle loading to after we check the default ALC) highlights that the satellite assembly tests were passing for the wrong reasons. The full fix here handles satellite loads correctly, putting them in the same ALC as the parent assembly. One non-satellite test that was previously passing also passed erroneously, but it makes no sense on wasm so disable it.

I don't know how likely we think customers are to use custom ALCs on wasm, but the current behavior seems fairly broken. For the repro I use `Assembly.Load(byte[])` to match the customer issue, but I think you can achieve the same effect by just loading an assembly into a custom ALC and trying to cast the same way.

If we do want to backport, the problem is these changes are all on top of @safern's previous bundle work, so I'd either need to backport the whole thing or manually implement some of this work on top of RC2. Maybe the latter option is the way to go here? Not sure how to proceed. cc: @marek-safar @SamMonoRT
  • Loading branch information
CoffeeFlux committed Sep 18, 2020
1 parent 163f458 commit cea7e01
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 24 deletions.
3 changes: 2 additions & 1 deletion mono/metadata/assembly-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
#else
#define MONO_ASSEMBLY_CORLIB_NAME "System.Private.CoreLib"
#endif
#define MONO_ASSEMBLY_CORLIB_RESOURCE_NAME (MONO_ASSEMBLY_CORLIB_NAME ".resources")
#define MONO_ASSEMBLY_RESOURCE_SUFFIX ".resources"
#define MONO_ASSEMBLY_CORLIB_RESOURCE_NAME (MONO_ASSEMBLY_CORLIB_NAME MONO_ASSEMBLY_RESOURCE_SUFFIX)

/* Flag bits for mono_assembly_names_equal_flags (). */
typedef enum {
Expand Down
72 changes: 49 additions & 23 deletions mono/metadata/assembly.c
Original file line number Diff line number Diff line change
Expand Up @@ -1651,16 +1651,16 @@ load_reference_by_aname_individual_asmctx (MonoAssemblyName *aname, MonoAssembly
}
#else
static MonoAssembly *
search_bundle_for_assembly (MonoAssemblyLoadContext *alc, MonoAssemblyName *aname, gboolean is_satellite)
search_bundle_for_assembly (MonoAssemblyLoadContext *alc, MonoAssemblyName *aname)
{
if ((bundles == NULL && !is_satellite) || (satellite_bundles == NULL && is_satellite))
if (bundles == NULL && satellite_bundles == NULL)
return NULL;

MonoImageOpenStatus status;
MonoImage *image;
MonoAssemblyLoadRequest req;
image = mono_assembly_open_from_bundle (alc, aname->name, &status, FALSE, aname->culture);
if (!image) {
if (!image && !g_str_has_suffix (aname->name, ".dll")) {
char *name = g_strdup_printf ("%s.dll", aname->name);
image = mono_assembly_open_from_bundle (alc, name, &status, FALSE, aname->culture);
}
Expand Down Expand Up @@ -1692,23 +1692,27 @@ netcore_load_reference (MonoAssemblyName *aname, MonoAssemblyLoadContext *alc, M
* Try these until one of them succeeds (by returning a non-NULL reference):
* 1. Check if it's already loaded by the ALC.
*
* 2. If we have a bundle registered, search the images for a matching name.
* 2. If it's a non-default ALC, call the Load() method.
*
* 3. If it's a non-default ALC, call the Load() method.
*
* 4. If the ALC is not the default and this is not a satellite request,
* 3. If the ALC is not the default and this is not a satellite request,
* check if it's already loaded by the default ALC.
*
* 5. If the ALC is the default or this is not a satellite request,
* 4. If we have a bundle registered and this is not a satellite request,
* search the images for a matching name.
*
* 5. If we have a satellite bundle registered and this is a satellite request,
* find the parent ALC and search the images for a matching name and culture.
*
* 6. If the ALC is the default or this is not a satellite request,
* check the TPA list, APP_PATHS, and ApplicationBase.
*
* 6. If this is a satellite request, call the ALC ResolveSatelliteAssembly method.
* 7. If this is a satellite request, call the ALC ResolveSatelliteAssembly method.
*
* 7. Call the ALC Resolving event.
* 8. Call the ALC Resolving event.
*
* 8. Call the ALC AssemblyResolve event (except for corlib satellite assemblies).
* 9. Call the ALC AssemblyResolve event (except for corlib satellite assemblies).
*
* 9. Return NULL.
* 10. Return NULL.
*/

reference = mono_assembly_loaded_internal (alc, aname, FALSE);
Expand All @@ -1717,14 +1721,6 @@ netcore_load_reference (MonoAssemblyName *aname, MonoAssemblyLoadContext *alc, M
goto leave;
}

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;
}
}

if (!is_default) {
reference = mono_alc_invoke_resolve_using_load_nofail (alc, aname);
if (reference) {
Expand All @@ -1741,6 +1737,38 @@ netcore_load_reference (MonoAssemblyName *aname, MonoAssemblyLoadContext *alc, M
}
}

if (bundles != NULL && !is_satellite) {
reference = search_bundle_for_assembly (mono_domain_default_alc (mono_alc_domain (alc)), aname);
if (reference) {
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Assembly found in the bundle: '%s'.", aname->name);
goto leave;
}
}

if (satellite_bundles != NULL && is_satellite) {
// Satellite assembly byname requests should be loaded in the same ALC as their parent assembly
size_t name_len = strlen (aname->name);
char *parent_name = NULL;
MonoAssemblyLoadContext *parent_alc = NULL;
if (g_str_has_suffix (aname->name, MONO_ASSEMBLY_RESOURCE_SUFFIX))
parent_name = g_strdup_printf ("%s.dll", g_strndup (aname->name, name_len - strlen (MONO_ASSEMBLY_RESOURCE_SUFFIX)));

if (parent_name) {
MonoAssemblyOpenRequest req;
mono_assembly_request_prepare_open (&req, MONO_ASMCTX_DEFAULT, alc);
MonoAssembly *parent_assembly = mono_assembly_request_open (parent_name, &req, NULL);
parent_alc = mono_assembly_get_alc (parent_assembly);
}

if (parent_alc)
reference = search_bundle_for_assembly (parent_alc, aname);

if (reference) {
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Assembly found in the satellite bundle: '%s'.", aname->name);
goto leave;
}
}

if (is_default || !is_satellite) {
reference = invoke_assembly_preload_hook (mono_domain_default_alc (mono_alc_domain (alc)), aname, assemblies_path);
if (reference) {
Expand Down Expand Up @@ -2593,11 +2621,9 @@ mono_assembly_open_from_bundle (MonoAssemblyLoadContext *alc, const char *filena
#else
gboolean is_satellite = culture && culture [0] != 0;;
if (is_satellite)
{
image = open_from_satellite_bundle (alc, filename, status, refonly, culture);
} else {
else
image = open_from_bundle_internal (alc, filename, status, refonly, FALSE);
}
#endif

if (image) {
Expand Down

0 comments on commit cea7e01

Please sign in to comment.