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.

Sign up for GitHub

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

Jump to bottom

Enable hosts to provide custom assembly resolution #73185

Merged
merged 10 commits into from
May 9, 2024
Merged

Enable hosts to provide custom assembly resolution #73185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b157716
Allow a host to override how the AnalyzerAssemblyLoader resolves asse…
chsienki Apr 22, 2024
e9b1a25
Add tests
chsienki Apr 22, 2024
706bea9
Fixup workspaces layer
chsienki Apr 22, 2024
55af5cf
Add a specific Razor assembly resolver in external accces
chsienki Apr 22, 2024
ebb7260
Fix top level imports
chsienki Apr 22, 2024
7c8ef76
Formatting
chsienki Apr 23, 2024
30ab5d4
Make compiler assembly loading a resolver in core loader
chsienki Apr 24, 2024
5544942
Remove unused alc parameter
chsienki Apr 24, 2024
8aa64f0
Remove uncessary default value from ctor
chsienki Apr 25, 2024
7ae01a3
PR Feedback:
chsienki Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
121 changes: 117 additions & 4 deletions src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public enum AnalyzerTestKind
///
/// Limitation 1: .NET Framework probing path.
///
/// The .NET Framework assembly loader will only call AppDomain.AssemblyResolve when it cannot satifisfy a load
/// The .NET Framework assembly loader will only call AppDomain.AssemblyResolve when it cannot satisfy a load
/// request. One of the places the assembly loader will always consider when looking for dependencies of A.dll
/// is the directory that A.dll was loading from (it's added to the probing path). That means if B.dll is in the
/// same directory then the runtime will silently load it without a way for us to intervene.
Expand Down Expand Up @@ -95,25 +95,27 @@ public AnalyzerAssemblyLoaderTests(ITestOutputHelper testOutputHelper, AssemblyL

#if NETCOREAPP

private void Run(AnalyzerTestKind kind, Action<AnalyzerAssemblyLoader, AssemblyLoadTestFixture> testAction, [CallerMemberName] string? memberName = null) =>
private void Run(AnalyzerTestKind kind, Action<AnalyzerAssemblyLoader, AssemblyLoadTestFixture> testAction, IAnalyzerAssemblyResolver[]? externalResolvers = null, [CallerMemberName] string? memberName = null) =>
Run(
kind,
static (_, _) => { },
testAction,
externalResolvers,
memberName);

private void Run(
AnalyzerTestKind kind,
Action<AssemblyLoadContext, AssemblyLoadTestFixture> prepLoadContextAction,
Action<AnalyzerAssemblyLoader, AssemblyLoadTestFixture> testAction,
IAnalyzerAssemblyResolver[]? externalResolvers = null,
[CallerMemberName] string? memberName = null)
{
var alc = new AssemblyLoadContext($"Test {memberName}", isCollectible: true);
try
{
prepLoadContextAction(alc, TestFixture);
var util = new InvokeUtil();
util.Exec(TestOutputHelper, alc, TestFixture, kind, testAction.Method.DeclaringType!.FullName!, testAction.Method.Name);
util.Exec(TestOutputHelper, alc, TestFixture, kind, testAction.Method.DeclaringType!.FullName!, testAction.Method.Name, externalResolvers ?? []);
}
finally
{
Expand All @@ -126,6 +128,7 @@ public AnalyzerAssemblyLoaderTests(ITestOutputHelper testOutputHelper, AssemblyL
private void Run(
AnalyzerTestKind kind,
Action<AnalyzerAssemblyLoader, AssemblyLoadTestFixture> testAction,
IAnalyzerAssemblyResolver[]? externalResolvers = null,
[CallerMemberName] string? memberName = null)
{
AppDomain? appDomain = null;
Expand All @@ -135,7 +138,7 @@ public AnalyzerAssemblyLoaderTests(ITestOutputHelper testOutputHelper, AssemblyL
var testOutputHelper = new AppDomainTestOutputHelper(TestOutputHelper);
var type = typeof(InvokeUtil);
var util = (InvokeUtil)appDomain.CreateInstanceAndUnwrap(type.Assembly.FullName, type.FullName);
util.Exec(testOutputHelper, TestFixture, kind, testAction.Method.DeclaringType.FullName, testAction.Method.Name);
util.Exec(testOutputHelper, TestFixture, kind, testAction.Method.DeclaringType.FullName, testAction.Method.Name, externalResolvers ?? []);
}
finally
{
Expand Down Expand Up @@ -1421,5 +1424,115 @@ public void AssemblyLoadingInNonDefaultContext_AnalyzerReferencesSystemCollectio
});
}
#endif

[Theory]
[CombinatorialData]
public void ExternalResolver_CanIntercept_ReturningNull(AnalyzerTestKind kind)
{
var resolver = new TestAnalyzerAssemblyResolver(n => null);
Run(kind, (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) =>
{
loader.AddDependencyLocation(testFixture.Delta1);
Assembly delta = loader.LoadFromPath(testFixture.Delta1);
Assert.NotNull(delta);
VerifyDependencyAssemblies(loader, testFixture.Delta1);

}, externalResolvers: [resolver]);
Assert.Collection(resolver.CalledFor, (a => Assert.Equal("Delta", a.Name)));
}

[Theory]
[CombinatorialData]
public void ExternalResolver_CanIntercept_ReturningAssembly(AnalyzerTestKind kind)
{
var resolver = new TestAnalyzerAssemblyResolver(n => GetType().Assembly);
Run(kind, (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) =>
{
// net core assembly loader checks that the resolved assembly name is the same as the requested one
// so we use the assembly the tests are contained in as its already be loaded
var thisAssembly = typeof(AnalyzerAssemblyLoaderTests).Assembly;
loader.AddDependencyLocation(thisAssembly.Location);
Assembly loaded = loader.LoadFromPath(thisAssembly.Location);
Assert.Equal(thisAssembly, loaded);

}, externalResolvers: [resolver]);
Assert.Collection(resolver.CalledFor, (a => Assert.Equal(GetType().Assembly.GetName().Name, a.Name)));
}

[Theory]
[CombinatorialData]
public void ExternalResolver_CanIntercept_ReturningAssembly_Or_Null(AnalyzerTestKind kind)
{
var thisAssemblyName = GetType().Assembly.GetName();
var resolver = new TestAnalyzerAssemblyResolver(n => n == thisAssemblyName ? GetType().Assembly : null);
Run(kind, (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) =>
{
var thisAssembly = typeof(AnalyzerAssemblyLoaderTests).Assembly;

loader.AddDependencyLocation(testFixture.Alpha);
Assembly alpha = loader.LoadFromPath(testFixture.Alpha);
Assert.NotNull(alpha);

loader.AddDependencyLocation(thisAssembly.Location);
Assembly loaded = loader.LoadFromPath(thisAssembly.Location);
Assert.Equal(thisAssembly, loaded);

loader.AddDependencyLocation(testFixture.Delta1);
Assembly delta = loader.LoadFromPath(testFixture.Delta1);
Assert.NotNull(delta);

}, externalResolvers: [resolver]);
Assert.Collection(resolver.CalledFor, (a => Assert.Equal("Alpha", a.Name)), a => Assert.Equal(thisAssemblyName.Name, a.Name), a => Assert.Equal("Delta", a.Name));
}
chsienki marked this conversation as resolved.
Show resolved Hide resolved

[Theory]
[CombinatorialData]
public void ExternalResolver_MultipleResolvers_CanIntercept_ReturningNull(AnalyzerTestKind kind)
{
var resolver1 = new TestAnalyzerAssemblyResolver(n => null);
var resolver2 = new TestAnalyzerAssemblyResolver(n => null);
Run(kind, (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) =>
{
loader.AddDependencyLocation(testFixture.Delta1);
Assembly delta = loader.LoadFromPath(testFixture.Delta1);
Assert.NotNull(delta);
VerifyDependencyAssemblies(loader, testFixture.Delta1);

}, externalResolvers: [resolver1, resolver2]);
Assert.Collection(resolver1.CalledFor, (a => Assert.Equal("Delta", a.Name)));
Assert.Collection(resolver2.CalledFor, (a => Assert.Equal("Delta", a.Name)));
}

[Theory]
[CombinatorialData]
public void ExternalResolver_MultipleResolvers_ResolutionStops_AfterFirstResolve(AnalyzerTestKind kind)
{
var resolver1 = new TestAnalyzerAssemblyResolver(n => GetType().Assembly);
var resolver2 = new TestAnalyzerAssemblyResolver(n => null);
Run(kind, (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) =>
{
var thisAssembly = typeof(AnalyzerAssemblyLoaderTests).Assembly;
loader.AddDependencyLocation(thisAssembly.Location);
Assembly loaded = loader.LoadFromPath(thisAssembly.Location);
Assert.Equal(thisAssembly, loaded);

}, externalResolvers: [resolver1, resolver2]);
Assert.Collection(resolver1.CalledFor, (a => Assert.Equal(GetType().Assembly.GetName().Name, a.Name)));
Assert.Empty(resolver2.CalledFor);
}

[Serializable]
private class TestAnalyzerAssemblyResolver(Func<AssemblyName, Assembly?> func) : MarshalByRefObject, IAnalyzerAssemblyResolver
{
private readonly Func<AssemblyName, Assembly?> _func = func;

public List<AssemblyName> CalledFor { get; } = [];

public Assembly? ResolveAssembly(AssemblyName assemblyName)
{
CalledFor.Add(assemblyName);
return _func(assemblyName);
}
}
}
}
14 changes: 7 additions & 7 deletions src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace Microsoft.CodeAnalysis.UnitTests

public sealed class InvokeUtil
{
public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compilerContext, AssemblyLoadTestFixture fixture, AnalyzerTestKind kind, string typeName, string methodName)
internal void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compilerContext, AssemblyLoadTestFixture fixture, AnalyzerTestKind kind, string typeName, string methodName, IAnalyzerAssemblyResolver[] externalResolvers)
{
// Ensure that the test did not load any of the test fixture assemblies into
// the default load context. That should never happen. Assemblies should either
Expand All @@ -48,9 +48,9 @@ public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compile
using var tempRoot = new TempRoot();
AnalyzerAssemblyLoader loader = kind switch
{
AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromDisk),
AnalyzerTestKind.LoadStream => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromStream),
AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(compilerContext, tempRoot.CreateDirectory().Path),
AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromDisk, externalResolvers.ToImmutableArray()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

micro-nit: it feels reasonable for these APIs to take ImmutableArray if this input array is invariably going to be copied to immutable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't because this call is used via .net remoting which doesn't support ImmutableArray :(

AnalyzerTestKind.LoadStream => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromStream, externalResolvers.ToImmutableArray()),
AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(compilerContext, tempRoot.CreateDirectory().Path, externalResolvers.ToImmutableArray()),
_ => throw ExceptionUtilities.Unreachable()
};

Expand Down Expand Up @@ -93,13 +93,13 @@ public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compile

public sealed class InvokeUtil : MarshalByRefObject
{
public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadTestFixture fixture, AnalyzerTestKind kind, string typeName, string methodName)
internal void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadTestFixture fixture, AnalyzerTestKind kind, string typeName, string methodName, IAnalyzerAssemblyResolver[] externalResolvers)
{
using var tempRoot = new TempRoot();
AnalyzerAssemblyLoader loader = kind switch
{
AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(),
AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(tempRoot.CreateDirectory().Path),
AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(externalResolvers.ToImmutableArray()),
AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(tempRoot.CreateDirectory().Path, externalResolvers.ToImmutableArray()),
_ => throw ExceptionUtilities.Unreachable()
};

Expand Down
50 changes: 31 additions & 19 deletions src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,16 @@ internal partial class AnalyzerAssemblyLoader
internal AssemblyLoadContext CompilerLoadContext => _compilerLoadContext;
internal AnalyzerLoadOption AnalyzerLoadOption => _loadOption;

internal AnalyzerAssemblyLoader()
: this(null, AnalyzerLoadOption.LoadFromDisk)
internal AnalyzerAssemblyLoader(ImmutableArray<IAnalyzerAssemblyResolver> externalResolvers)
: this(null, AnalyzerLoadOption.LoadFromDisk, externalResolvers)
{
}

internal AnalyzerAssemblyLoader(AssemblyLoadContext? compilerLoadContext, AnalyzerLoadOption loadOption)
internal AnalyzerAssemblyLoader(AssemblyLoadContext? compilerLoadContext, AnalyzerLoadOption loadOption, ImmutableArray<IAnalyzerAssemblyResolver> externalResolvers)
{
_loadOption = loadOption;
_compilerLoadContext = compilerLoadContext ?? AssemblyLoadContext.GetLoadContext(typeof(AnalyzerAssemblyLoader).GetTypeInfo().Assembly)!;
_externalResolvers = [.. externalResolvers, new CompilerAnalyzerAssemblyResolver(_compilerLoadContext)];
chsienki marked this conversation as resolved.
Show resolved Hide resolved
}

public bool IsHostAssembly(Assembly assembly)
Expand All @@ -69,7 +70,7 @@ public bool IsHostAssembly(Assembly assembly)
{
if (!_loadContextByDirectory.TryGetValue(fullDirectoryPath, out loadContext))
{
loadContext = new DirectoryLoadContext(fullDirectoryPath, this, _compilerLoadContext);
loadContext = new DirectoryLoadContext(fullDirectoryPath, this);
_loadContextByDirectory[fullDirectoryPath] = loadContext;
}
}
Expand Down Expand Up @@ -107,33 +108,23 @@ internal sealed class DirectoryLoadContext : AssemblyLoadContext
{
internal string Directory { get; }
private readonly AnalyzerAssemblyLoader _loader;
private readonly AssemblyLoadContext _compilerLoadContext;

public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader, AssemblyLoadContext compilerLoadContext)
public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader)
: base(isCollectible: true)
{
Directory = directory;
_loader = loader;
_compilerLoadContext = compilerLoadContext;
}

protected override Assembly? Load(AssemblyName assemblyName)
{
var simpleName = assemblyName.Name!;
try
if (_loader.ResolveAssemblyExternally(assemblyName) is { } externallyResolvedAssembly)
chsienki marked this conversation as resolved.
Show resolved Hide resolved
{
if (_compilerLoadContext.LoadFromAssemblyName(assemblyName) is { } compilerAssembly)
{
return compilerAssembly;
}
}
catch
{
// Expected to happen when the assembly cannot be resolved in the compiler / host
// AssemblyLoadContext.
return externallyResolvedAssembly;
}

// Prefer registered dependencies in the same directory first.
var simpleName = assemblyName.Name!;
var assemblyPath = Path.Combine(Directory, simpleName + ".dll");
if (_loader.IsAnalyzerDependencyPath(assemblyPath))
{
Expand All @@ -147,7 +138,7 @@ public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader, Ass
// Note: when loading from disk the .NET runtime has a fallback step that will handle
// satellite assembly loading if the call to Load(satelliteAssemblyName) fails. This
// loader has a mode where it loads from Stream though and the runtime will not handle
// that automatically. Rather than bifurate our loading behavior between Disk and
// that automatically. Rather than bifurcate our loading behavior between Disk and
// Stream both modes just handle satellite loading directly
if (assemblyName.CultureInfo is not null && simpleName.EndsWith(".resources", StringComparison.Ordinal))
{
Expand Down Expand Up @@ -201,6 +192,27 @@ protected override IntPtr LoadUnmanagedDll(string unmanagedDllName)
return IntPtr.Zero;
}
}

/// <summary>
/// A resolver which allows a passed in <see cref="AssemblyLoadContext"/> from the compiler
/// to control assembly resolution. This is important because there are many exchange types
/// that need to unify across the multiple analyzer ALCs. These include common types from
/// <c>Microsoft.CodeAnalysis.dll</c> etc, as well as platform assemblies provided by a
/// host such as visual studio.
/// </summary>
/// <remarks>
/// This resolver essentially forces any assembly that was loaded as a 'core' part of the
/// compiler to be shared across analyzers, and not loaded multiple times into each individual
/// analyzer ALC, even if the analyzer itself shipped a copy of said assembly.
/// </remarks>
/// <param name="compilerContext">The <see cref="AssemblyLoadContext"/> that the core
/// compiler assemblies are already loaded into.</param>
internal sealed class CompilerAnalyzerAssemblyResolver(AssemblyLoadContext compilerContext) : IAnalyzerAssemblyResolver
chsienki marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly AssemblyLoadContext _compilerAlc = compilerContext;

public Assembly? ResolveAssembly(AssemblyName assemblyName) => _compilerAlc.LoadFromAssemblyName(assemblyName);
}
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#if !NETCOREAPP

using System;
using System.Collections.Immutable;
using System.Globalization;
using System.IO;
using System.Reflection;
Expand All @@ -28,8 +29,9 @@ internal partial class AnalyzerAssemblyLoader
{
private bool _hookedAssemblyResolve;

internal AnalyzerAssemblyLoader()
internal AnalyzerAssemblyLoader(ImmutableArray<IAnalyzerAssemblyResolver> externalResolvers)
{
_externalResolvers = externalResolvers;
}

public bool IsHostAssembly(Assembly assembly)
Expand Down Expand Up @@ -57,6 +59,10 @@ public bool IsHostAssembly(Assembly assembly)
private partial Assembly? Load(AssemblyName assemblyName, string assemblyOriginalPath)
{
EnsureResolvedHooked();
if (ResolveAssemblyExternally(assemblyName) is { } externallyResolvedAssembly)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this needs to go after EnsureReslovedHooked. Consider the case where:

  1. myanalyzer.dll and util.dll are passed via /analyzer:
  2. myanalyzer.dll depends on util.dll such that it's required when roslyn reads types from the assembly
  3. There is a resolver that hooks myanalyzer.dll but nothing else

This version of the code would end up throwing. That is because the load from myanalyzer.dll would come from an external location. Normal assembly resolution won't find util.dll (becuase the resolver didn't load myanalyzer.dll from a place that had it). The AssemblyResolve method would find it but it won't run because resolution hasn't been hooked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't manage to construct a test in a way that allows us to show this behavior, but I think I can convince myself that hooking first is the correct order.

{
return externallyResolvedAssembly;
}

return AppDomain.CurrentDomain.Load(assemblyName);
}
Expand Down