Microsoft.NET.StringTools not properly loading into separate ALC

[JanKrivanek]

Context

This is based on the #9538
Microsoft.NET.StringTools is not being loaded into separate AssemblyLoadContext properly in the C# DevKit leading to regression issues in case of changes in functionality of StringTools.

Details

The Microsoft.NET.StringTools.dll is located in the sdk folder along with the othe MSBuild assemblies that are trying to load it, however it fails to be loaded into nondefault ACL, fallbacking to the default ACL which already has StringTools loaded into separate ACL:

Possible investigation steps

1. Load Microsoft.Build.dll into custom Assembly Load Context
2. Execute code that will lead to pulling other binaries - e.g. simple evaluation of a Project should do:

var path = "some\csrpoj\path.csproj";
var toolsVersion = "Current";
var xml = Microsoft.Build.Construction.ProjectRootElement.Open(path);
var rootProj = new Microsoft.Build.Evaluation.Project(xml, Microsoft.Build.Evaluation.ProjectCollection.GlobalProjectCollection.GlobalProperties, toolsVersion);

3. Inspect the ACL being used for all involved MSBuild assemblies. If you decide to use ETW for investigation, you can:

   1. Collect the counters via (need to be started before the scenario execution and ctrl-c after scenario done):

   .\PerfView.exe collect /NoGui /Providers=Microsoft-Windows-DotNETRuntime /NoNGenRundown /Merge:False /DataFile:msbuild-alc-etw /BufferSize:8096 /CircularMB:8096
   

   2. And then filter-in the Microsoft-Windows-DotNETRuntime/AssemblyLoader EventTypes and Microsoft.NET.StringTools (and possbily Microsoft.Build as well) data. For easier viewing you can setup AssemblyName ActivityID Success * columns.

   

[Forgind]

I only briefly glanced at this issue, but is CSDK using MSBuildLocator? MSBuildLocator specifies loading our assemblies into the default ALC:
https://github.com/microsoft/MSBuildLocator/blob/0bd7d2689d9e0e03a4b0e5bcdccf053f61887f90/src/MSBuildLocator/MSBuildLocator.cs#L253

[JanKrivanek]

I only briefly glanced at this issue, but is CSDK using MSBuildLocator? MSBuildLocator specifies loading our assemblies into the default ALC: https://github.com/microsoft/MSBuildLocator/blob/0bd7d2689d9e0e03a4b0e5bcdccf053f61887f90/src/MSBuildLocator/MSBuildLocator.cs#L253

I do not know. But it's a good lead indeed! Should definitely be investigated as part of this item.

From my uninformed point of view - forcing the Default ALC should be used only if necessary and in such case accompanied with checking and leading to explicit fast failing if attempted to workaround. Though at this point such ship might already have sailed.

[Forgind]

We're generally more willing to accept breaking changes in MSBuildLocator than MSBuild proper, but we can also avoid a breaking change if we have a separate API that permits you to specify your ALC and falls back to the default 🙂 That isn't quite the same as what you're proposing, but if that really is the root of the problem, CSDK should be able to opt in, which would mitigate the issue from their perspective.

[ladipro]

The root cause of this issue is in the CoreCLR assembly loader itself. Apparently when an assembly is being loaded into a secondary ALC but another assembly with the same name is already loaded into the default ALC, the Resolving event for the secondary ALC is not called and the assembly from the default ALC is reused.

@vitek-karas, is this by design? It's caught us by surprise here and cascaded into a user impacting issue in #9538.

Here's a simple repro, which I believe reflects what's happening in C# Dev Kit (paths may need to be updated):
ALCTwoAssemblies.zip

[ladipro]

Making an explicit LoadFromAssemblyPath call with the desired path "fixes" it in that subsequent by-name binds get correctly resolved to this explicitly loaded assembly.

alc.LoadFromAssemblyPath(@"C:\temp\ALCTwoAssemblies\ClassLib2\bin\Debug\net8.0\ClassLib.dll");

This could potentially be a workaround for CDK.

[Forgind]

LoadFromAssemblyPath leads to more JITting, doesn't it? Relative to Load?

Is it intentional for it to be added to the default ALC? Another possible option might be to ensure it's never added to the default ALC and use multiple not-default ALCs, though that might be a bit unwieldy.