MSBuild on .NET Core does not allow tasks with same assembly name, but different assembly versions

[nguerrera]

With desktop MSBuild / VS, we've gotten around issues with node reuse and varying versions of the SDK by increasing the assembly version of the SDK tasks on every build. (Aside: we've regressed that a bunch of times with infrastructure changes.)

However, this does not work on .NET Core. I suspect we haven't noticed this until now because node reuse has only made its way to .NET Core recently.

Steps to reproduce

Run ReproCore.cmd from nguerrera/repros@fc1e1df

WARNING: First step of repro kills msbuild.exe processes, so don't do this with real builds in progress in parallel.

Command line

git clone https://github.com/nguerrera/repros
cd repros
git checkout fc1e1df38786edff4258c6c2ed09ec6f4c89c4a3
cd NodeReuseAndAssemblyVer
ReproCore.cmd

Expected behavior

Final build command in .cmd succeeds as it does on Desktop (try ReproDesktop.cmd instead).

Actual behavior

D:\Src\repros\NodeReuseAndAssemblyVer\repro.proj(16,9): error MSB4062: The "MyTask" task could not be loaded from the assembly D:\Src\repros\NodeReuseAndAssemblyVer\bin\2.0.0\netstandard2.0\MyTask.dll.
Assembly with same name is already loaded Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements
Microsoft.Build.Framework.ITask.

Environment data

Microsoft (R) Build Engine version 15.8.160-preview+gaf9d27ba72 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

15.8.160.40367

.NET Core SDK (reflecting any global.json):
 Version:   2.1.400-preview-009171
 Commit:    6f5d38734b

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17134
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.1.400-preview-009171\

cc @davkean @rainersigwald @dsplaisted

[nguerrera]

Hmm, this repro might not be good because the assembly isn't strong name signed. I suspect desktop is silently reloading old version. Let me update it to strong name sign the assembly...

[nguerrera]

Repro fixed to strong name task. Still gives the unexpected behavior on core.

[rainersigwald]

Huh. This check seems like it'd do it, but I haven't debugged at all

https://github.com/Microsoft/msbuild/blob/901c77484df3eece289829c431542d69c0294999/src/Shared/CoreCLRAssemblyLoader.cs#L126-L132

[nguerrera]

This may be a limitation of putting all tasks in default assembly load context.

[rainersigwald]

That's plausible. This throws:

System.IO.FileLoadException: Assembly with same name is already loaded
   at System.Runtime.Loader.AssemblyLoadContext.LoadFromPath(IntPtr ptrNativeAssemblyLoadContext, String ilPath, String niPath, ObjectHandleOnStack retAssembly)
   at System.Runtime.Loader.AssemblyLoadContext.LoadFromAssemblyPath(String assemblyPath)
   at Microsoft.Build.Shared.CoreClrAssemblyLoader.LoadAndCache(String fullPath) in /_/src/Shared/CoreCLRAssemblyLoader.cs:line 146
   at Microsoft.Build.Shared.CoreClrAssemblyLoader.LoadFromPath(String fullPath) in /_/src/Shared/CoreCLRAssemblyLoader.cs:line 72
   at Microsoft.Build.Shared.TypeLoader.LoadAssembly(AssemblyLoadInfo assemblyLoadInfo) in /_/src/Shared/TypeLoader.cs:line 181
   at Microsoft.Build.Shared.TypeLoader.AssemblyInfoToLoadedTypes.ScanAssemblyForPublicTypes() in /_/src/Shared/TypeLoader.cs:line 370
   at Microsoft.Build.Shared.TypeLoader.AssemblyInfoToLoadedTypes.<>c__DisplayClass8_0.<GetLoadedTypeByTypeName>b__0(String key) in /_/src/Shared/TypeLoader.cs:line 341
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Microsoft.Build.Shared.TypeLoader.AssemblyInfoToLoadedTypes.GetLoadedTypeByTypeName(String typeName) in /_/src/Shared/TypeLoader.cs:line 314
   at Microsoft.Build.Shared.TypeLoader.GetLoadedType(ConcurrentDictionary`2 cache, String typeName, AssemblyLoadInfo assembly) in /_/src/Shared/TypeLoader.cs:line 242
   at Microsoft.Build.Shared.TypeLoader.Load(String typeName, AssemblyLoadInfo assembly) in /_/src/Shared/TypeLoader.cs:line 208
   at Microsoft.Build.BackEnd.AssemblyTaskFactory.InitializeFactory(AssemblyLoadInfo loadInfo, String taskName, IDictionary`2 taskParameters, String taskElementContents, IDictionary`2 taskFactoryIdentityParameters, Boolean taskHostFactoryExplicitlyRequested, TargetLoggingContext targetLoggingContext, ElementLocation elementLocation, String taskProjectFile) in /_/src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs:line 278

And indeed, from https://github.com/dotnet/coreclr/blob/master/Documentation/design-docs/assemblyloadcontext.md#custom-loadcontext

Multiple assemblies with the same simple name cannot be loaded into a single load context (Default or Custom). Also, .Net Core ignores strong name token for assembly binding process.

So I guess we should have a load context per . . . assembly path?

[nguerrera]

So I guess we should have a load context per . . . assembly path?

Yes. More precisely, per UsingTask AssemblyFile path. Dependencies of the task would come into the same load context as the task.

This is very much related to the challenges that @natemcmaster outlined here: https://natemcmaster.com/blog/2018/07/25/netcore-plugins/

Many things he has mentioned have bit msbuild loading tasks and roslyn loading analyzers in turn.

cc @jeffschwMSFT

[nguerrera]

I guess this comes down to more motivation for #1754

[dasMulli]

I'm currently using something similar to https://github.com/AArnott/Nerdbank.MSBuildExtension/blob/master/src/Nerdbank.MSBuildExtension/netstandard1.5/ContextIsolatedTask.cs
to work around conflicting references.

What popped up on SO was that loading assemblies that are also used by the SDK (newtonsoft.json) are impossible to load (without a custom AssemblyLoadContext to execute your logic).

TL;DR having a load context per task assembly in general would be great! (#1754)

[JamesNK]

What's the status of this bug? Customers are encountering problems with the build tasks in Grpc.Tools:

grpc/grpc#20501

[nguerrera]

I believe #4916 would address this and planned for 16.5 / sdk 3.1.200, right, @rainersigwald ?

[rainersigwald]

Correct; I'll close this as a duplicate.

[rainersigwald]

Duplicate of #1754