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

There's no way to call native library resolution on a specific AssemblyLoadContext #13472

Open
vitek-karas opened this issue Sep 24, 2019 · 12 comments

Comments

@vitek-karas
Copy link
Member

Currently it's possible to create a "chain" of AssemblyLoadContext instances such that there's a ParentALC and ChildALC. ChildALC has a reference to ParentALC and every time the ChildALC.Load is called, if it can't resolve the assembly it falls back to parent by calling ParentALC.LoadFromAssemblyName (which will in turn call ParentALC.Load and so on).

It is not possible to do the same for native library resolution. While the child can overload ChildALC.LoadUnmanagedDll, there's no API it can call on the ParentALC to fall back to its native library resolution. There might be some workarounds using contextual reflection and NativeLibrary but that would be rather tricky and absolutely non-discoverable.

Given the current API design, it would probably be cleaner to add a new API to NativeLibrary - something like IntPtr NativeLibrary.Load(string libraryName, AssemblyLoadContext context). Or alternatively expose something similar on the ALC itself.

@albahari
Copy link

albahari commented Nov 15, 2019

I've also run into this problem, and believe we need to add a public LoadFromUnmanagedDllName method to AssemblyLoadContext.

But there's a deeper issue.

ALCs are essential for isolation & dynamic loading because we no longer have AppDomains.

However, isolation & dynamic loading is (almost) useless if it relies on libraries being written in a special way to make them "isolation-friendly" or "dynamic-load-friendly": Even if you write an assembly specifically to be isolated, chances are that you'll have at least one NuGet dependency that hasn't been written specially to be isolated.

At a minimum, one should fall into the pit of success when using .NET Core in a canonical fashion. A library that you write should execute correctly in a non-default ALC without special programming.

The difficulty with NativeLibrary.Load (when called with a simple name) is that it bypasses the resolution logic of the executing assembly's ALC, breaking the ALC isolation model. It's likely that any library that uses NativeLibrary.Load (e.g, Microsoft.Data.Sqlite, which calls SQLitePCLRaw.nativelibrary), will break when loaded into a non-default ALC or when loaded dynamically. Ironically, a major purpose of NativeLibrary is to allow the name of the native library to be chosen at runtime (i.e., dynamically). And yet, when its assembly that calls NativeLibrary is loaded dynamically (into a custom ALC), it breaks!

In fixing this, I believe there are two important principles:

  • Pit of success for non-default ALCs. Using .NET Core in a canonical fashion shouldn't lead to code that can't run in a custom ALC.
  • Where possible, symmetry between the APIs in their handling of managed vs native DLLs (principle of least surprise).

Here's what I propose:

  1. Expose a new public method on AssemblyLoadContext called LoadFromUnmanagedDllName. This would call the protected LoadUnmanagedDll method and be the unmanaged equivalent of LoadFromAssemblyName, providing consistency between managed and unmanaged.
  2. Change the behavior of NativeLibrary.Load in .NET Core 5 so that it calls the LoadFromUnmanagedDllName method of the executing assembly's ALC. This makes it consistent with the way that Assembly.Load(string) works, helping with managed/unmanaged API consistency. This also provides consistency with the way DllImport works: it resolves using the executing assembly's ALC. So it's not strange for NativeLibrary.Load to do the same.

I appreciate that there are subtle issues with Assembly.Load(string) inferring the ALC (which have led to ContextualReflectionContext). But the problem stems from the ALC being implicit rather than explicit; not from the wrong default choice of ALC. If we were to change Assembly.Load so that it always used the default ALC, the situation would get far worse. So given that we're stuck with Assembly.Load(string):

  • Executing ALC = good (current behavior)
  • Default ALC = bad

NativeLibrary.Load (string) is analogous. The ALC is also implicit rather than explicit. Like Assembly.Load(string), we can't take it away. The issue is, what is the best default choice for the inferred ALC? The situation is

  • Executing ALC = good
  • Default ALC = bad (current behavior)

I understand that NativeLibrary was intended as a low-level API. But what does this mean? It's just another way of saying that it doesn't invoke the current ALC for resolution (begging the question) and also that it's technically valid to call NativeLibrary from an ALC's LoadUnmanagedDll method. But why would you do this, when you already have LoadUnmanagedDllFromPath right there in front of you - along with examples that tell you to call LoadUnmanagedDllFromPath?

Note that this entire issue applies to when you're calling NativeLibrary.Load with a simple name (in other words, when you're asking it to resolve). In this situation, the library author is taking responsibility for what DLL to load, while allowing the application to determine where the DLL is located.

This should be fixed in .NET Core 5, before too many API authors start using NativeLibrary. Each time they do, they're unintentionally writing a library that will break in a non-default ALC.

@jkotas
Copy link
Member

jkotas commented Nov 15, 2019

  • NativeLibrary.Load(string) is a low-level API that wraps the underlying library load OS API and nothing else. It is meant to be used e.g. for loading OS and similar libraries that are outside the application.
  • NativeLibrary.Load(string libraryName, Assembly assembly, DllImportSearchPath? searchPath) is a higher level API. The intention for this API was to behave as if DllImport was used in the given assembly.

The problem seems to be that NativeLibrary.Load(string libraryName, Assembly assembly, DllImportSearchPath? searchPath) does not behave as DllImport in the given assembly. I believe that the cleanest fix may be to change NativeLibrary.Load(string libraryName, Assembly assembly, DllImportSearchPath? searchPath) to call AssemblyLoadContext.LoadUnmanagedDll and AssemblyLoadContext.ResolvingUnmanagedDll. No new API needed. Thoughts?

@jkotas
Copy link
Member

jkotas commented Nov 15, 2019

It is not possible to do the same for native library resolution. While the child can overload ChildALC.LoadUnmanagedDll, there's no API it can call on the ParentALC to fall back to its native library resolution.

The native library is an implementation detail of an assembly. Thus the ALC that actually loaded the assembly should be the one responsible for loading its native libraries too. What is a real example where the two ALC are different and the chaining is needed?

The chaining problem and the problem that @albahari desribed do not seem to be same issues.

@vitek-karas
Copy link
Member Author

I agree that the chaining problem is not the same as @albahari states - they're just related as they are around the same APIs. And if we're considering adding/modifying public APIs, it's worth to have most of the related issues around it together.

The chaining problem is a hypothetical scenario where there are two levels of plugins. AssemblyLoadContext does not naturally create hierarchy - all ALCs are peers except for Default which is "parent" for everything else. Because of this, for the multi-level plugin scenario the custom ALCs need to manually create the hierarchy, by calling the "parent" from the "child" explicitly. This is possible for managed assemblies as the necessary APIs are public. It's currently not possible for native libraries.

That said I do agree that this should be a niche case - as you mention in vast majority of cases the native dependencies of an assembly should be resolved by the ALC into which the assembly is loaded (as that is logically how it is constructed). So this mostly comes into play for explicit loads like the NativeLibrary.Load(string, Assembly, DllImportSearchPath?) (once it's fixed) - or for "weird" scenarios where the plugins want to explicitly share native dependencies as well.

@albahari
Copy link

albahari commented Nov 15, 2019

I believe that the cleanest fix may be to change NativeLibrary.Load(string libraryName, Assembly assembly, DllImportSearchPath? searchPath) to call AssemblyLoadContext.LoadUnmanagedDll and AssemblyLoadContext.ResolvingUnmanagedDll. No new API needed. Thoughts?

Yes, that would be a big step forward and solve at least half the problem.

But an issue would remain: it's far too easy to make a mistake and call NativeLibrary.Load("foo") instead of NativeLibrary.Load("foo", Assembly.GetExecutingAssembly(), null) or NativeLibrary.Load("foo", typeof(MyWrapper).Assembly, null). The difficulty is that unless the library author tests their code outside the default ALC, they'll never know that there's a problem.

I can see two ways to fix the remaining issue:

  1. Have NativeLibrary.Load(string lib) call NativeLibrary.Load(lib, Assembly.GetCallingAssembly(), null) so that by default, the executing ALC gets a chance to resolve.
  2. Make NativeLibrary.Load(string) behave as intended, and only resolve OS and libraries that are outside the application. Then if they called NativeLibrary.Load("foo") - and foo was a DLL that ships with the application - it would crash no matter what ALC they are using. This would then force them into using the overload that accepts an assembly and that honors that assembly's ALC.

(2) is cleaner, but more likely to break existing code.

@jkotas
Copy link
Member

jkotas commented Nov 15, 2019

NativeLibrary.Load(string) behave as intended, and only resolve OS and libraries that are outside the application.

The intended behavior of NativeLibrary.Load is to just call the underlying OS API. On Unix, the native library probing paths do not include the program directory by default. On Windows, the native library probing paths do include the .exe directory by default. This API is not trying to hide OS differences.

@albahari
Copy link

In that case, I misunderstood: it's not looking at the default ALC's TPA paths, just the .exe directory.

So then your suggestion of changing NativeLibrary.Load(string libraryName, Assembly assembly, DllImportSearchPath? searchPath) to call the assembly's AssemblyLoadContext.LoadUnmanagedDll and AssemblyLoadContext.ResolvingUnmanagedDll would work nicely.

@albahari
Copy link

Should I log a new issue for this?

i.e., changing the behaviour of NativeLibrary.Load(string,Assembly,DllImportSearchPath)?

@vitek-karas
Copy link
Member Author

@albahari Feel free to do that - I think in the end we might do only that - since that would more or less fix the issue mentioned in this one...

@John0King
Copy link

anyone hits the Managed Assemblies's Alc across issue ;
in my library https://github.com/John0King/LazyMan.ModularLoader, I want to side load the whole web site as subapp , then I facing this Assemblies's Alc across issue,
it too complex, that I can not reproduce this issue simple, but when you use my lib to run, there are at least 98% web app will crash.

a little explain: (this is after a long time debug)

  1. all Microsoft.AspNetCore.App assemblies is load in default ALC by default , in this case my lib works
  2. if you reference/separate to multiple class library (such as DTO, model, service layer), then some of the Microsoft.AspNetCore.App assemblies will in web app's local folder, so those .dll well load in pluginAlc , notice: not all will copy to local folder.
  3. anything you call in your primary dll (from pluginAlc) is ok, but remember the powerfull of DI in asp.net core. many configuration call is in Microsoft.AspNetCore.App 's assemblise, so the issue happend.
  4. (such as Microsoft.Extensions.DependencyInject.Abstraction.dll in pluginAlc , let's called p_core.dll for shot , and core.dll for those in default alc) because p_core.dll is in pluginAlc, and some DI configure call in default alc, , now core.dll call will looking p_core.dll 's class in default alc, that cause MethodNotFoundException

why not load everything in pluginAlc?

I can't , because the AssemblyDependencyResolver can not located the core.dll

why not load all core.dll in default Alc?

I can't by default , because it in the .deps.json , and will be discovered by AssemblyDependencyResolver

reprduce

run https://github.com/John0King/LazyMan.ModularLoader/tree/master/test/SubAppTest the HostApp, and copy a bigger webapp (just not black app) for example OrchardCore (need to change the CreateWebHost to CreateWebHostBuilder)

@vitek-karas
Copy link
Member Author

@John0King Is your problem related to this issue? As far as I can tell you don't have any native components in your project. This issue is only about problems with handling native dependencies in hierarchical ALCs - neither of which you seem to be using. It would be better to create a new issue for this.

That aside I think that trying to load ASP.NET app into a secondary ALC is going to be problematic. I've seen issues around this already. Mostly they're because ASP.NET (and the libraries it uses) are not ALC aware and sometimes do things which will lead to problems. Good example is here:

var appAssembly = Assembly.Load(new AssemblyName(env.ApplicationName));
- since Microsoft.Extensions.Hosting is going to be loaded to the default ALC (it's part of a framework) the Assembly.Load will work on the default ALC only - so if you try to load an application into a secondary ALC this will either directly fail, or it will not work as expected. I'm not sure how many other samples are there in ASP.NET of this.

There might be some tricks you can play with CurrentContextualReflectionContext but it's not guaranteed to fix the problem. In short I think this is a problem in ASP.NET which was simply not designed to work in secondary ALCs - at least yet.

@John0King
Copy link

I'm sorry ask here, but I really do not know where should I create a new issue, runtime/aspnetcore ,
the reason I post here is because I think it should be a alc fullback issue:

defualt alc:   A --(call) --> B  --(call)---> C
plugin alc :    D--(call)-> [  A  ](fallback from default alc)    --🚫--> C ′ 

in plugin alc, assembly D in plugin alc can call assembly A from defualt alc , but then it can not call the C ′ from plugin alc, because the A is in default alc, and it can only find C instead of C′,
the theory to solve this, is make the type/assembly finding by CurrentContextualReflectionContext

so when D call A it is in plugin alc Context, and when find C , it will look plugin alc first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

5 participants