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

Dependencies isolation bug in AssemblyLoadContext #87578

Closed
hyperion-cs opened this issue Jun 14, 2023 · 9 comments
Closed

Dependencies isolation bug in AssemblyLoadContext #87578

hyperion-cs opened this issue Jun 14, 2023 · 9 comments

Comments

@hyperion-cs
Copy link

Description

If you meet the following conditions:

  1. You use AssemblyLoadContext to dynamically load modules (.dll);
  2. The parent project (entry point) uses a third-party nuget package X;
  3. One of the module projects (which are loaded in step 1) uses a nuget package which in turn has an internal dependency on the nuget package X (i.e. the same one used in step 2).

Then the dependency isolation of dynamically loaded modules is broken. It contains TWO references to package X in its context of loaded assemblies. Thus, further work with this package is impossible, because we will get exceptions like "A cannot be cast to A" at any attempt to access the package's functionality. The text of the exception may change depending on the conditions (e.g., to "method does not exist"), but the meaning remains the same.

This seems to be a common enough case that it should be fixed immediately.

Reproduction Steps

I have created a minimal project that reproduces the problem:
https://github.com/hyperion-cs/net-assemblyloadcontext-isolation-problem

Csla version 7.0.0 is used as the problematic third-party nuget package. However, this is not crucial. ANY nuget package that has internal dependencies matching the parent (EntryPoint) project can be used in the same way. For example, Telegram.Bots.Extensions.Polling version 5.9.0 may also suit.

As you can notice, after building and running the project you will get an exception of the "A cannot be cast to A" kind. However, you only need to remove the reference to the third-party nuget package from NET_AssemblyLoadContext_IsolationProblem.Lib.csproj:

- <PackageReference Include="Csla" Version="7.0.0" />

And also by removing using to it from LibClass.cs:

- using Csla;

Then everything will definitely work as expected (of course, do NOT forget to restore nuget packages, clean up the libext folder in the Debug output folder of the EntryPoint project (it should not contain Csla.dll, etc.), and rebuild the entire solution). Note that after the build, AfterBuild is used to copy the extension files.

Expected behavior

Program output:

LibClass: I did something!
Done.

Actual behavior

The throwing out of the exception:

[A]Microsoft.Extensions.Logging.Logger`1[NET_AssemblyLoadContext_IsolationProblem.LibBase.ILib] cannot be cast to
[B]Microsoft.Extensions.Logging.Logger`1[NET_AssemblyLoadContext_IsolationProblem.LibBase.ILib].
Type A originates from 'Microsoft.Extensions.Logging.Abstractions, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' in the context 'Default' at location '/Users/***/Projects/NET_AssemblyLoadContext_IsolationProblem/net-assemblyloadcontext-isolation-problem/NET_AssemblyLoadContext_IsolationProblem.EntryPoint/bin/Debug/net7.0/Microsoft.Extensions.Logging.Abstractions.dll'.
Type B originates from 'Microsoft.Extensions.Logging.Abstractions, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' in the context '"" NET_AssemblyLoadContext_IsolationProblem.EntryPoint.ProblemLoadContext #0' at location '/Users/***/Projects/NET_AssemblyLoadContext_IsolationProblem/net-assemblyloadcontext-isolation-problem/NET_AssemblyLoadContext_IsolationProblem.EntryPoint/bin/Debug/net7.0/libext/Microsoft.Extensions.Logging.Abstractions.dll'.

Regression?

This problem seems to have existed since the development of AssemblyLoadContext.

Known Workarounds

As a temporary solution, you can track in ProblemLoadContext.Load(...) the re-loading of problematic dependencies and skip them. But this is a very dirty and bad solution. Example:

// We can also compare the version, etc.
if(assemblyName.Name = "Microsoft.Extensions.Logging.Abstractions")
{
    return null; // Skip loading.
}

Configuration

.NET Version: 7.0
OS: macOS 13.4
Architecture: x64

The same problem in Linux/Windows, so the current configuration is not so important.

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 14, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 14, 2023
@ghost
Copy link

ghost commented Jun 14, 2023

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

Issue Details

Description

If you meet the following conditions:

  1. You use AssemblyLoadContext to dynamically load modules (.dll);
  2. The parent project (entry point) uses a third-party nuget package X;
  3. One of the module projects (which are loaded in step 1) uses a nuget package which in turn has an internal dependency on the nuget package X (i.e. the same one used in step 2).

Then the dependency isolation of dynamically loaded modules is broken. It contains TWO references to package X in its context of loaded assemblies. Thus, further work with this package is impossible, because we will get exceptions like "A cannot be cast to A" at any attempt to access the package's functionality. The text of the exception may change depending on the conditions (e.g., to "method does not exist"), but the meaning remains the same.

This seems to be a common enough case that it should be fixed immediately.

Reproduction Steps

I have created a minimal project that reproduces the problem:
https://github.com/hyperion-cs/net-assemblyloadcontext-isolation-problem

Csla version 7.0.0 is used as the problematic third-party nuget package. However, this is not crucial. ANY nuget package that has internal dependencies matching the parent (EntryPoint) project can be used in the same way. For example, Telegram.Bots.Extensions.Polling version 5.9.0 may also suit.

As you can notice, after building and running the project you will get an exception of the "A cannot be cast to A" kind. However, you only need to remove the reference to the third-party nuget package from NET_AssemblyLoadContext_IsolationProblem.Lib.csproj:

- <PackageReference Include="Csla" Version="7.0.0" />

And also by removing using to it from LibClass.cs:

- using Csla;

Then everything will definitely work as expected (of course, do NOT forget to restore nuget packages, clean up the libext folder in the Debug output folder of the EntryPoint project (it should not contain Csla.dll, etc.), and rebuild the entire solution). Note that after the build, AfterBuild is used to copy the extension files.

Expected behavior

Program output:

LibClass: I did something!
Done.

Actual behavior

The throwing out of the exception:

[A]Microsoft.Extensions.Logging.Logger`1[NET_AssemblyLoadContext_IsolationProblem.LibBase.ILib] cannot be cast to
[B]Microsoft.Extensions.Logging.Logger`1[NET_AssemblyLoadContext_IsolationProblem.LibBase.ILib].
Type A originates from 'Microsoft.Extensions.Logging.Abstractions, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' in the context 'Default' at location '/Users/***/Projects/NET_AssemblyLoadContext_IsolationProblem/net-assemblyloadcontext-isolation-problem/NET_AssemblyLoadContext_IsolationProblem.EntryPoint/bin/Debug/net7.0/Microsoft.Extensions.Logging.Abstractions.dll'.
Type B originates from 'Microsoft.Extensions.Logging.Abstractions, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' in the context '"" NET_AssemblyLoadContext_IsolationProblem.EntryPoint.ProblemLoadContext #0' at location '/Users/***/Projects/NET_AssemblyLoadContext_IsolationProblem/net-assemblyloadcontext-isolation-problem/NET_AssemblyLoadContext_IsolationProblem.EntryPoint/bin/Debug/net7.0/libext/Microsoft.Extensions.Logging.Abstractions.dll'.

Regression?

This problem seems to have existed since the development of AssemblyLoadContext.

Known Workarounds

As a temporary solution, you can track in ProblemLoadContext.Load(...) the re-loading of problematic dependencies and skip them. But this is a very dirty and bad solution. Example:

// We can also compare the version, etc.
if(assemblyName.Name = "Microsoft.Extensions.Logging.Abstractions")
{
    return null; // Skip loading.
}

Configuration

.NET Version: 7.0
OS: macOS 13.4
Architecture: x64

The same problem in Linux/Windows, so the current configuration is not so important.

Other information

No response

Author: hyperion-cs
Assignees: -
Labels:

area-AssemblyLoader-coreclr, untriaged, needs-area-label

Milestone: -

@teo-tsirpanis teo-tsirpanis removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 14, 2023
@davidfowl
Copy link
Member

davidfowl commented Jun 19, 2023

This is very common and if dependencies bleed from one context into the other then you will end up with problems. When using load contexts, dependencies that cross the boundary between contexts need to be unified. So, in your example project, plugins cannot load their own version of the logging abstractions since that dependency is already going to be provided by the host.

If you're just trying to load assemblies dynamically into the main process there's no need for a custom load context, just use the default one. Otherwise, you should think about any plugin system like this:

  • What dependencies does the host have?
  • What does my host provide to plugins?
  • What dependencies can a plugin have?

Since you're using the DI container, I assume dependencies added to it from the host, will flow into the plugins. You need to account for that when determining where to load assemblies from (the plugin or host).

In the image below, any type passed to Plugin A (in your example ILogger) from the host needs to come from the same place.

image

In your example:

image

Your plugin is using Microsoft.Extensions.Loggging from Csla in the pluginloadcontext and the host is using Microsoft.Extensions.Loggging from the default load context.

@hyperion-cs
Copy link
Author

@davidfowl, thank you very much for your reply! I really appreciate it.
I think I now understand the causes of the problem, but I have not figured out how to solve it (using my example project).
I need a modular system where modules can be written by third-party developers (for example). The problem is that I cannot manage internal dependencies, which have module dependencies (plugin A in the diagram). For example, the internal CSLA dependencies in the diagram above are a black box, there is nothing I can do about them. In other words, there is no way I can prevent CSLA from using Microsoft.Extensions.Logging internally. At the same time, my module (plugin A in the diagram), is obliged to get ILogger<T> from a Host that also uses Microsoft.Extensions.Logging.
How do I solve exactly this confusion? I can't refuse to use ILogger<T> from the Host, because it's the host's responsibility to provide such logger. I can't refuse CSLA (and other nuget packages that can use Microsoft.Extensions.Logging) in the module (plugin A) either, for obvious reasons.
Perhaps I can somehow explicitly select in the module (plugin A) which assembly I need ILogger<T> from? I haven't found such a way. Or maybe I'm thinking completely wrong and this can somehow be easily fixed?

@davidfowl
Copy link
Member

Private dependencies can truly be private, but then you can't use the DI container to provide them which wants to share dependencies. You don't need to control what dependencies other packages used, but you need to unify (aka move them to shared) any dependencies that are loaded or reference by the host.

You need to have a list of assemblies that are loaded by the host and then you need to add logic here:

https://github.com/hyperion-cs/net-assemblyloadcontext-isolation-problem/blob/31ec0d8fade53abe7b4104cb756c1b0945e48e98/NET_AssemblyLoadContext_IsolationProblem.EntryPoint/ProblemLoadContext.cs#L20

To detect if one of those assemblies are being loaded, then you need to delegate to AssemblyLoadContext.Default.

@hyperion-cs
Copy link
Author

@davidfowl, so, let's commit the solution for "descendants" (those who will read this issue). Am I correct in my understanding that in general, if the requirements specified by you are met, a check like this (or some similar logic) in .Load(...) of own AssemblyLoadContext is sufficient:

protected override Assembly Load(AssemblyName assemblyName)
{
    // Check if such the assembly has already been loaded by Host.
    if (AssemblyLoadContext.Default.Assemblies
            .FirstOrDefault(x => x.FullName == assemblyName.FullName) is not null)
    {
        return null;
    }

    var assemblyPath = _resolver.ResolveAssemblyToPath(assemblyName);
    if (assemblyPath != null)
    {
        return LoadFromAssemblyPath(assemblyPath);
    }

    return null;
}

This helped in my project and now everything works. However, I want to make sure that I solved the problem adequately, without using "smelly code", etc.

@vitek-karas
Copy link
Member

Unfortunately this is not a reliable way to fix this problem. The AssemblyLoadContext.Default.Assemblies collection only returns already loaded assemblies. If there's a dependency of the host which the host hasn't used yet, it may not be in the collection. But later on, it may appear there and if the custom ALC decided to isolate it into a plugin as well, you would run into the same problem again.

The correct way to do this is to write something like:

try
{
    var asm = AssemblyLoadContext.Default.LoadFromAssemblyName(name);
    if (asm != null)
    {
        return asm;
    }
}
catch
{
   // Assembly is not part of the host - load it into the plugin
}

// Load the dependency from the plugin

There's some more discussion around the same topic here: #87185

Just to expand on David's response. For the plugin isolation to work correctly, any types involved in the contract between the host and plugin must NOT be isolated, instead, those types should always be provided by the host. In simple cases the contract is the interface the plugin implements, and the host uses to call into the plugin, and so it's relatively easy to reason about what is in it. But shared global state is also part of the contract, even though it's not explicitly visible. DI is an example of a shared global state (the service registrations are the shared state) and so it effectively extends the contract to include all of the types used in the DI (or at least the subset used or provided by the plugin).

One final point (since we've had a long discussion on this recently internally): The usage of AssemblyLoadContext.Default may or may not be correct either. If the code which implements the custom AssemblyLoadContext is always loaded by the host application directly into the Default ALC, then it's correct and everything will work. But if the plugin host may also be hosted by something else, then its ALC may not be Default in which case the code should use the ALC of the plugin host instead. It's important to remember that ALCs are not a hierarchy, all ALCs are peers except for Default, which is a "parent" to all other ALCs.

@hyperion-cs
Copy link
Author

@vitek-karas, thank you so much for your reply! Everything seems to be falling into place now. I think we can now close the current issue.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jun 20, 2023
@davidfowl
Copy link
Member

@vitek-karas we should write this up somewhere with a better diagram than mine 😄

@hyperion-cs
Copy link
Author

hyperion-cs commented Jun 21, 2023

@davidfowl, guys, that's a really good idea. Because before I created the current issue, I literally searched all over the Internet trying to find an adequate solution to my problem. Creating this issue was a last resort.
There is enough information here for a whole article, and it is very useful knowledge/experience.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants