dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

Have implicit DLL loading rules changed between .NET Core 1.0 and 1.1? #2673

Closed bradwilson closed 4 years ago

bradwilson commented 7 years ago

Background

I work on xUnit.net. I'm writing a console test runner which is deployed as a .NET CLI command line tool.

The runner shim works by finding the unit test DLL (after building), and then issuing a dotnet exec to run a .NET Core port of our console runner, based on code and help I've gotten from @natemcmaster. In order to ensure that everything is loadable, I copy all the DLLs for the runner and its supporting libraries into the bin folder, and then run the dotnet exec using that bin folder as the working directory (code here). That allows me to implicitly load my runner and support libraries without having them be linked into the unit test project.

This strategy is working for netcoreapp1.0 projects, but one of my support DLLs is failing to load with netcoreapp1.1. The DLL that fails to load is present in the bin folder. I asked Nate to look over my execution logs, but he didn't find anything.

Steps to reproduce

  1. Clone https://github.com/xunit/xunit.integration
  2. From the v2x_netcoreapp1.0 folder, run dotnet restore, then dotnet xunit. The tests should run successfully (there is one failing test). Output looks like this:
Detecting target frameworks in v2x_netcoreapp1.0.csproj...
Building for framework netcoreapp1.0...
  v2x_netcoreapp1.0 -> C:\Dev\xunit\xunit.integration\v2x_netcoreapp1.0\bin\Debug\netcoreapp1.0\v2x_netcoreapp1.0.dll
Running .NET Core tests for framework netcoreapp1.0...
xUnit.net Console Runner (64-bit .NET Core 4.0.0.0)
  Discovering: v2x_netcoreapp1.0 (method display = ClassAndMethod)
  Discovered:  v2x_netcoreapp1.0 (running 3 test cases)
  Starting:    v2x_netcoreapp1.0 (parallel test collections = on, max threads = 8)
    BasicTests.Failing [FAIL]
      Assert.True() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        C:\Dev\xunit\xunit.integration\v2x_netcoreapp1.0\BasicTests.cs(14,0): at BasicTests.Failing()
    BasicTests.Theory(value: null) [FAIL]
      Assert.NotNull() Failure
      Stack Trace:
        C:\Dev\xunit\xunit.integration\v2x_netcoreapp1.0\BasicTests.cs(23,0): at BasicTests.Theory(Object value)
  Finished:    v2x_netcoreapp1.0
=== TEST EXECUTION SUMMARY ===
   v2x_netcoreapp1.0  Total: 5, Errors: 0, Failed: 2, Skipped: 0, Time: 0.171s
  1. From the v2x_netcoreapp1.1 folder, run dotnet restore then dotnet xunit. The tests fail to run with a FileNotFoundException:
Detecting target frameworks in v2x_netcoreapp1.1.csproj...
Building for framework netcoreapp1.1...
  v2x_netcoreapp1.1 -> C:\Dev\xunit\xunit.integration\v2x_netcoreapp1.1\bin\Debug\netcoreapp1.1\v2x_netcoreapp1.1.dll
Running .NET Core tests for framework netcoreapp1.1...

Unhandled Exception: System.IO.FileNotFoundException: Could not load file or assembly 'xunit.runner.utility.netcoreapp10, Version=2.3.0.3633, Culture=neutral, PublicKeyToken=8d05b1bb7a6fdb6c'. The system cannot find the file specified.
   at Xunit.ConsoleClient.Program.Main(String[] args)

All the DLLs I expect to be in the bin folder are there, including the one that would not load:

[...]
xunit.abstractions.dll
xunit.assert.dll
xunit.console.dll
xunit.console.runtimeconfig.json
xunit.core.dll
xunit.execution.dotnet.dll
xunit.runner.json
xunit.runner.reporters.netcoreapp10.dll
xunit.runner.utility.netcoreapp10.dll
[...]

Expected behavior

Tests run successfully.

Actual behavior

Tests fail to run because of FileNotFoundException.

Environment data

PS> dotnet

Microsoft .NET Core Shared Framework Host

  Version  : 1.1.0
  Build    : 928f77c4bc3f49d892459992fb6e1d5542cb5e86
[...]

PS> dotnet --version
1.0.1
bradwilson commented 7 years ago

Here is a gist with the output from COREHOST_TRACE=1: https://gist.github.com/bradwilson/bf2e06679cd0992416e8654b618cf6d1

bradwilson commented 7 years ago

/tag @eerhardt on the advice of Nate McMaster

eerhardt commented 7 years ago

The xunit.runner.utility.netcoreapp10 assembly is not listed in the .deps.json file of the test, which is why it is failing to load.

This was an explicit bug fix/change from netcoreapp1.0 to netcoreapp1.1. It was a bug in 1.0 that assemblies in the app folder were getting loaded without being in the .deps.json file.

I think the issue that fixed it was https://github.com/dotnet/core-setup/issues/193, but @gkhanna79 and @ramarag would know exactly which one it was.

To fix this, either:

  1. The test project needs to depend on all the assemblies that it intends to load. That way the assemblies make it into the .deps.json file, and the host will load them.
  2. xunit implements an AssemblyLoadContext, which calls the various Load* methods in order to get the correct assembly to load.
natemcmaster commented 7 years ago

It was a bug in 1.0 that assemblies in the app folder were getting loaded without being in the .deps.json file.

🤔 Interesting.... I was under the impression this was how it was supposed to work.

Any file with the suffix .dll in the same folder as the managed application being loaded (the "Application Base") will be considered a viable assembly during the resolution process.

cref https://github.com/dotnet/core-setup/blob/master/Documentation/design-docs/corehost.md#files-in-the-application-folder

gkhanna79 commented 7 years ago

Yes, that is an incorrect statement that should have been fixed but was not. @ramarag Can you please update the document?

bradwilson commented 7 years ago

@eerhardt Do you have any documentation/examples of how I would implement an AssemblyLoadContext?

gkhanna79 commented 7 years ago

See https://github.com/dotnet/coreclr/blob/master/Documentation/design-docs/assemblyloadcontext.md

bradwilson commented 7 years ago

@gkhanna79 Thanks!

Closing now, since this is expected behavior.

bradwilson commented 7 years ago

@gkhanna79 I'm unclear after reading the documentation as to how you replace the default load context (since the things that are failing to load are implicit dependencies). How is this achieved?

bradwilson commented 7 years ago

I made it work using the default load context's Resolving event. Thanks!