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.

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

Asp.Net 5.0 Blazor Assembly.Load does not create types that link to types in loaded assemblies #42207

Closed
allderidge opened this issue Sep 11, 2020 · 8 comments · Fixed by #42425 or mono/mono#20406
Assignees
Milestone

Comments

@allderidge
Copy link

Describe the bug

When using Blazor running in the browser with dotnet 5.0.0-preview.8.20407.11 an assembly loaded using Assembly.Load(byte[] rawAssembly, byte[]? rawSymbolStore) will create types that reference assemblies that appear to be duplicates of assemblies that have already been loaded preventing any use of shared types. This behavior is different to other non WASM runtimes and to the dotnet 3.2.0 WASM runtime.

The bug is not present in the release of WASM Blazor in dotnet core 3.2.0 and was found during an upgrade of existing code.

E.g.

  • A Type A implements interface IA
  • Interface IA is defined in an assembly that the a Blazor component is build against
  • Type A is defined in an project that references the project interface IA is present in
  • Interface IA will be resolved when the Blazor component loads.
  • Type A is defined in an assembly loaded by Assembly.Load
  • In the dotnet 5.0 WASM runtime the interface IA implemented by Type A is defined in an assembly that is different to the interface IA referenced by the Blazor component meaning that the component cannot cast Type A to Interface IA.
  • The same code run in a stardard dotnet 5.0 runtime is correctly able to perform the cast.

To Reproduce

A reproduction based on the Blazor starter project is available from: https://github.com/allderidge/BlazorBugReport_AssemblyLoad

There are two solutions:

  • AssemblyLoadTest_5_0/AssemblyLoadTest.sln Reproduces the bug and illustrates that a test in a standard runtime passes.
  • AssemblyLoadTest_3_2_0/AssemblyLoadTest.sln Illustrates the bug is not present in the dotnet core 3.2

The web page shown running in dotnet 5.0:

image

The web page shown running in dotnet core 3.2

image

Illustrative code

var assemblyData = await Client.GetFromJsonAsync<DynamicAssembly>("Assembly");
var assembly = Assembly.Load(assemblyData.AssemblyDllBytes, assemblyData.AssemblyPdbBytes);

var dynamicType = assembly.GetType("AssemblyLoadTest.Dynamic.SharedInterfaceImplementation", true) ?? throw new ApplicationException("Unable to load dynamic type");
var sharedInterface = dynamicType.GetInterfaces().First(e => e.Name == nameof(ISharedInterface));
SharedAssemblyMatches = sharedInterface.Assembly == typeof(ISharedInterface).Assembly;  // should be true, false in bug repo
SharedInterfaceMatches = sharedInterface == typeof(ISharedInterface); // should be true, false in bug repo

var instance = Activator.CreateInstance(dynamicType);
CanCast = instance is ISharedInterface;  // should be true, false in bug repo

Name = ((ISharedInterface)instance).HelloWorldName; // causes a invalid cast exception

Further technical details

  • ASP.NET Core version: 5.0.0-preview.8.20414.8
  • Include the output of dotnet --info
 Version:   5.0.100-preview.8.20417.9
 Commit:    fc62663a35

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18363
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.100-preview.8.20417.9\

Host (useful for support):
  Version: 5.0.0-preview.8.20407.11
  Commit:  bf456654f9

.NET SDKs installed:
  2.1.504 [C:\Program Files\dotnet\sdk]
  2.1.602 [C:\Program Files\dotnet\sdk]
  2.1.700-preview-009601 [C:\Program Files\dotnet\sdk]
  3.0.100-preview4-011223 [C:\Program Files\dotnet\sdk]
  3.1.103 [C:\Program Files\dotnet\sdk]
  3.1.200 [C:\Program Files\dotnet\sdk]
  3.1.300-preview-015048 [C:\Program Files\dotnet\sdk]
  3.1.302 [C:\Program Files\dotnet\sdk]
  3.1.402 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.8.20417.9 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview4-19216-03 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.0-preview.8.20414.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.22 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview4-27615-11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0-preview.8.20407.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview4-27613-28 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0-preview.8.20411.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version
    Microsoft Visual Studio Enterprise 2019 Preview Version 16.8.0 Preview 2.1
@pranavkm
Copy link
Contributor

I suspect you'll need to load things in to the default load context AssemblyLoadContext.Default.LoadFromStream. Assembly.Load in .NET Core loads assemblies in an assembly specific load context.

@allderidge
Copy link
Author

Using AssemblyLoadContext.Default.LoadFromStream as an alternative does successfully load the assembly so that a cast to a common interface can be performed.

I think the implementation of the custom AssemblyLoadContext created during Assembly.Load has an issue in that it should not be loading any other assemblies other than the one its given, finding other assemblies should delegate to AssemblyLoadContext.Default.

@mkArtakMSFT mkArtakMSFT transferred this issue from dotnet/aspnetcore Sep 14, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-AssemblyLoader-coreclr untriaged New issue has not been triaged by the area owner labels Sep 14, 2020
@ghost
Copy link

ghost commented Sep 14, 2020

Tagging subscribers to this area: @vitek-karas, @agocke
See info in area-owners.md if you want to be subscribed.

@lewing
Copy link
Member

lewing commented Sep 14, 2020

cc @CoffeeFlux

@ghost
Copy link

ghost commented Sep 14, 2020

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

@CoffeeFlux CoffeeFlux added the arch-wasm WebAssembly architecture label Sep 14, 2020
@CoffeeFlux
Copy link
Contributor

CoffeeFlux commented Sep 14, 2020

I assume this is the result of our bundle check being very early in the loading algorithm, so if the assembly isn't loaded in the custom ALC it tries the load it in via the bundle before checking the default context. The solution is probably to just move the bundle check later in the process, after we've checked the default ALC.

This is maybe worth backporting, so I'll have a fix up ASAP. I'll add a test as well. Thanks for such a detailed bug report, it makes a fix much easier!

@CoffeeFlux CoffeeFlux modified the milestones: 5.0.0, 6.0.0 Sep 16, 2020
monojenkins pushed a commit to monojenkins/mono that referenced this issue Sep 18, 2020
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
@CoffeeFlux
Copy link
Contributor

As a summary of things as they stand: I have a PR up that adds a libraries test for this, along with a fix. It's not a simple backport, and there are some concerns about a previous PR this builds on, so I'm going to talk things through with @lambdageek, decide how we want to approach this, and then backport as appropriate.

@CoffeeFlux
Copy link
Contributor

Backport has been merged for this. Waiting on mono/mono CI to merge the main PR.

CoffeeFlux added a commit to mono/mono that referenced this issue Sep 22, 2020
…20406)

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

Co-authored-by: CoffeeFlux <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.