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

[netcoreapp2.0] For an assembly loaded via 'Assembly.LoadFrom', the assembly it referenced cannot be loaded even if they are in the same folder #21286

Closed
daxian-dbw opened this issue Apr 23, 2017 · 26 comments
Labels
area-System.Reflection bug tenet-compatibility Incompatibility with previous versions or .NET Framework
Milestone

Comments

@daxian-dbw
Copy link
Contributor

This is a breaking change in behavior compared with full .NET.

Scenario
I have libraries Sample.dll and SampleDepend.dll in the same folder c:\temp, and Sample.dll depends on SampleDepend.dll. In my test.exe, I load Sample.dll via Assembly.LoadFrom, and then invoke a method from Sample.dll using reflection, which will trigger the implicit loading of SampleDepend.dll.

Expected
The implicit loading of SampleDepend.dll succeeds, just like how it works with full .NET.

Actual
Exception thrown: Could not load file or assembly 'SampleDepend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

Repro Steps

// test.exe
namespace Test {
    public class Program {
        public static void Main() {
            var a = System.Reflection.Assembly.LoadFrom(@"c:\temp\Sample.dll");
            var t = a.GetType("Sample.Module");
            var m = t.GetMethod("GetName");
            var r = m.Invoke(null, new object[0]);
            System.Console.WriteLine(r);
        }
    }
}

// Sample.dll
using System;
using SampleDepend;

namespace Sample
{
    public class Module
    {
        public static string GetName()
        {
            return Utils.GetName();
        }
    }
}

// SampleDepend.dll
using System;

namespace SampleDepend
{
    public class Utils
    {
        public static string GetName()
        {
            return "SampleDepend.Utils";
        }
    }
}

Rresult

PS:159> .\bin\Debug\netcoreapp2.0\win10-x64\publish\test.exe

Unhandled Exception: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.IO.FileNotFoundException: Could not load file or assembly 'SampleDepend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
   at Sample.Module.GetName()
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
   at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at Test.Program.Main() in C:\Users\rocky\arena\dotnet\test\Program.cs:line 8

Full .NET Behavior

c:\arena>Test.exe
SampleDepend.Utils

I think when Assembly.LoadFrom successfully loads an assembly from path, it should put the path of the directory in the probing paths of the underlying load context, so that an assembly in the same folder that is referenced by the loaded assembly can be successfully loaded when running code from the loaded assembly.

@karelz
Copy link
Member

karelz commented Apr 23, 2017

@atsushikan @DnlHarvey this looks like a breaking change against .NET Framework. Is it intentional?
If yes, why? Are there any mitigations/workarounds/guidance? Do we have it officially documented?

@ghost
Copy link

ghost commented Apr 24, 2017

This is a question for runtime folks. Reflection doesn't dictate the rules of the binder - it only provides api access to it. CoreCLR has its own assembly binder with different rules from the desktop binder (Fusion.)

@karelz
Copy link
Member

karelz commented Apr 24, 2017

@gkhanna79 who can help answer here? Especially on the decision if it is something intentional vs. compat bug for 2.0.

@gkhanna79
Copy link
Member

@daxian-dbw Please have a look at https://github.com/dotnet/coreclr/blob/master/Documentation/design-docs/assemblyloadcontext.md - LaodFrom loads into Default context for .NET Core.

so that an assembly in the same folder that is referenced by the loaded assembly can be successfully loaded

This has never been the design for .NET Core loader (which is where it differs from Desktop byd esign) - only artifacts in app.deps.json are part of the default load context. You will need to wireup to Resolving event to handle this case.

@daxian-dbw
Copy link
Contributor Author

@gkhanna79 thanks for the reply. I read the code of Assembly.LoadFrom implementation and understand why it behaves this way. But I think it's a bug that should be fixed because it's an unobvious behavior change that is hard to know before actually using this API and found the code broken.

Assembly.LoadFrom is so commonly used that I think many developers will use it in netcoreapp2.0 applications without too much knowledge about AssemblyLoadContext. It would be a surprise when they find this behavior change. If it's determined that this is by design, then the behavior changes should be well documented in the API references, so that developers can know this before using the API.

@gkhanna79
Copy link
Member

The behavior change is a function of the binder difference between Desktop and .NET Core, which exists by design. There is no concept of binding by paths as well, as a consequence.

the behavior changes should be well documented in the API references,

I agree with this.

CC @jkotas

@karelz
Copy link
Member

karelz commented Apr 24, 2017

At minimum we should add it here: https://github.com/dotnet/corefx/wiki/ApiCompat

@danmoseley
Copy link
Member

@joshfree can you please triage and close/move to future/2.1 or assign for ZBB

@ghost
Copy link

ghost commented Apr 26, 2017

Closing via gkhanna's explanation. This is the sort of mishap that happens when we bring in ill-fitting apis just to conform to a metadata surface area.

@ghost ghost closed this as completed Apr 26, 2017
@karelz
Copy link
Member

karelz commented Apr 26, 2017

@atsushikan did you document the breaking change in https://github.com/dotnet/corefx/wiki/ApiCompat?
If not, let's reopen this bug to track the documentation in 2.0 ...

@ghost
Copy link

ghost commented Apr 26, 2017

I'd prefer the runtime team document it. They have the full view of the binder behavior changes and workarounds. The api is just the guy that lets you in.

@karelz karelz reopened this Apr 26, 2017
@karelz
Copy link
Member

karelz commented Apr 26, 2017

OK, reopening -- can you please track down the right person to document it and assign it to that person / appropriate area?

@davidfowl
Copy link
Member

FWIW I brought this up when the API was added to .NET Core 2.0 and I think it should work. We should make LoadFrom at the very least, look at assemblies next to itself. That's like 80% of what people expect.

@karelz
Copy link
Member

karelz commented May 1, 2017

@jkotas @gkhanna79 did we consider emulating Desktop 'assemblies next to each other' behavior in CoreCLR? Is it technically possible in CoreCLR binding model?
Could we change default? (compat with 1.x may be a problem now :()
If not, could we provide easy workaround -> e.g. opt-in, or API overload, or simple API alternative?

cc @danmosemsft as this may cause some non-trivial migration pain

@gkhanna79
Copy link
Member

did we consider emulating Desktop 'assemblies next to each other' behavior

@jkotas can chime in if it was considered but there is no such thing that CoreCLR Binder supports. This is one of the places where .NET Core differs from Desktop significantly - other areas are things like binding redirection, lack of GAC, lack of AppDomains, etc. Even for LoadFrom, the API is not exactly equivalent to how it behaves in Desktop since, in .NET Core, we always load into DefaultContext (see https://blogs.msdn.microsoft.com/suzcook/2003/09/19/loadfile-vs-loadfrom/ for details).

@daxian-dbw
Copy link
Contributor Author

We tried to mimic Assembly.LoadFrom in powershell in .NET Core 1.1 and the prior versions, and the way we do is to have a probing path list keep track of the directory paths of the assemblies that are loaded by LoadFrom, and use the probing paths in override Load or Resolving handler so that implicit loadings via Assembly.Load can discover the dependency assemblies sitting next to the loaded ones.

Maybe the default load context can do something similar? It doesn't have to offer the exact same behavior as in desktop, but as @davidfowl mentioned, this one is really expected by most people.

@jkotas
Copy link
Member

jkotas commented May 1, 2017

And how do you resolve conflicting versions?

Just looking next to the assembly is not hard. It becomes hard once you get multiple assembly versions with same name and have to start picking the rigth one. The sole purpose of desktop fusion with all binding redirects, Load/LoadFrom contexts, promotion from LoadFrom to Load, etc. is to pick the right copy of the assembly to use in the given situation. CoreCLR loader does not implement any of this policy. It leaves it up to the app to define its own.

If apps want to simulate bits of the full .NET Framework assembly resolution policy, they can register AssemblyResolve event and implement it there with as many tweaks and hack as necessary to make their particular case work. We can have examples if there are common cases that emerge. It is not that different from what apps do on the full .NET Framework today - most apps that load plugins register for AssemblyResolve event because of the built-in one does not work for them anyway.

@karelz
Copy link
Member

karelz commented May 1, 2017

I like the idea of examples - we can easily call out limitations of each example, without taking on the burden of dealing with by-design bugs and long explanations why something behaves as it does.

@davidfowl would that be acceptable middle-ground?

@davidfowl
Copy link
Member

What if we just handle the "I have a bunch of assemblies side by side" issue? We basically do what everyone is going to do and on a call to LoadFrom we add a "probing path" where the target assembly is.

Can we start by talking about that specific problem to see where the edges are? I don't want to start with the "we have to mimic desktop behavior" because I don't think we do.

@karelz
Copy link
Member

karelz commented May 1, 2017

How would you address @jkotas's point of conflicts? Throw? Use the first one loaded?
That's the policy baking which feels a bit like a slippery slope IMO.

@davidfowl
Copy link
Member

We already have policy baked in today in CoreCLR, versions are unified (highest wins).

Let me flip the question, what happens today if you load another dll into the same load context of a higher or lower version?

@gkhanna79
Copy link
Member

By Design, a load has every only loaded a single version of the assembly, independent of whether it is Desktop or CoreCLR.

@karelz
Copy link
Member

karelz commented May 1, 2017

That's a good question. However we have a problem that "look next to the files" approach will NOT necessarily probe for already-loaded assemblies in new locations. Or do you propose to always probe to unsure no new conflicts were created?

@gkhanna79
Copy link
Member

@jkotas and I chatted about this offline. There is no good way to mimic the exact Desktop behavior without introducing more complexity and confusion to the resolution mechanics for the user. We agreed that we should document the following:

Assembly being loaded using LoadFrom must have its transitive closure as part of the app OR the app should wire-up to ResolvingEvent to resolve the dependencies as these are the only correct ways to address this scenario.

@davidfowl
Copy link
Member

@jkotas and I chatted about this offline. There is no good way to mimic the exact Desktop behavior without introducing more complexity and confusion to the resolution mechanics for the user. We agreed that we should document the following:

I still don't think you need the exact desktop behavior, we might be overthinking it. Can you explain the reasoning?

Assembly being loaded using LoadFrom must have its transitive closure as part of the app OR the app should wire-up to ResolvingEvent to resolve the dependencies as these are the only correct ways to address this scenario.

If we can't do something basic how can users....

@gkhanna79 Can we do more than write docs and instead write a sample?

@gkhanna79
Copy link
Member

Fixed with dotnet/coreclr#11333

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Reflection bug tenet-compatibility Incompatibility with previous versions or .NET Framework
Projects
None yet
Development

No branches or pull requests

7 participants