Merge pull request #1248 from microsoft/dev/andarno/fixDynamicAssemblies_main

Create dynamic proxies in the same ALC as the interface they implement

Author: AArnott

StreamJsonRpc.slnx

@@ -32,5 +32,6 @@
     <Project Path="test/StreamJsonRpc.Tests.ExternalAssembly/StreamJsonRpc.Tests.ExternalAssembly.csproj" />
     <Project Path="test/StreamJsonRpc.Tests.NoInterceptors/StreamJsonRpc.Tests.NoInterceptors.csproj" />
     <Project Path="test/StreamJsonRpc.Tests/StreamJsonRpc.Tests.csproj" />
+    <Project Path="test/UnreachableAssembly/UnreachableAssembly.csproj" />
   </Folder>
 </Solution>

docfx/docs/proxies.md

@@ -61,3 +61,32 @@ In which case, you can declare your server methods to also return @System.Thread
 Sometimes a client may need to block its caller until a response to a JSON-RPC request comes back.
 The proxy maintains the same async-only contract that is exposed by the <xref:StreamJsonRpc.JsonRpc> class itself.
 [Learn more about sending requests](sendrequest.md), particularly under the heading about async responses.
+
+## Dynamic proxies
+
+The following concerns are related specifically to dynamically generated proxies and do not apply to source generated proxies.
+
+### AssemblyLoadContext considerations
+
+When in a .NET process with multiple <xref:System.Runtime.Loader.AssemblyLoadContext> (ALC) instances, you should consider whether StreamJsonRpc is loaded in an ALC that can load all the types required by the proxy interface.
+
+By default, StreamJsonRpc will generate dynamic proxies in the ALC that the (first) interface requested for the proxy is loaded within.
+This is usually the right choice because the interface should be in an ALC that can resolve all the interface's type references.
+When you request a proxy that implements *multiple* interfaces, and if those interfaces are loaded in different ALCs, you *may* need to control which ALC the proxy is generated in.
+The need to control this may manifest as an <xref:System.MissingMethodException> or <xref:System.InvalidCastException> due to types loading into multiple ALC instances.
+
+In such cases, you may control the ALC used to generate the proxy by surrounding your proxy request with a call to <xref:System.Runtime.Loader.AssemblyLoadContext.EnterContextualReflection*> (and disposal of its result).
+
+For example, you might use the following code when StreamJsonRpc is loaded into a different ALC from your own code:
+
+```cs
+// Whatever ALC can resolve *all* type references in *all* proxy interfaces.
+AssemblyLoadContext alc = AssemblyLoadContext.GetLoadContext(MethodBase.GetCurrentMethod()!.DeclaringType!.Assembly);
+IFoo proxy;
+using (AssemblyLoadContext.EnterContextualReflection(alc))
+{
+    proxy = (IFoo)jsonRpc.Attach([typeof(IFoo), typeof(IFoo2)]);
+}
+```
+
+This initializes the `proxy` local variable with a proxy that will be able to load all types that your own <xref:System.Runtime.Loader.AssemblyLoadContext> can load.

src/StreamJsonRpc/AssemblyNameEqualityComparer.cs

@@ -0,0 +1,36 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+
+using System.Reflection;
+
+namespace StreamJsonRpc;
+
+internal class AssemblyNameEqualityComparer : IEqualityComparer<AssemblyName>
+{
+    internal static readonly IEqualityComparer<AssemblyName> Instance = new AssemblyNameEqualityComparer();
+
+    private AssemblyNameEqualityComparer()
+    {
+    }
+
+    public bool Equals(AssemblyName? x, AssemblyName? y)
+    {
+        if (x is null && y is null)
+        {
+            return true;
+        }
+
+        if (x is null || y is null)
+        {
+            return false;
+        }
+
+        return string.Equals(x.FullName, y.FullName, StringComparison.OrdinalIgnoreCase);
+    }
+
+    public int GetHashCode(AssemblyName obj)
+    {
+        Requires.NotNull(obj, nameof(obj));
+
+        return StringComparer.OrdinalIgnoreCase.GetHashCode(obj.FullName);
+    }
+}

src/StreamJsonRpc/ProxyGeneration.cs

@@ -6,6 +6,9 @@
 using System.Globalization;
 using System.Reflection;
 using System.Reflection.Emit;
+#if NET
+using System.Runtime.Loader;
+#endif
 using Microsoft.VisualStudio.Threading;
 using CodeGenHelpers = StreamJsonRpc.Reflection.CodeGenHelpers;
 
@@ -19,7 +22,11 @@ namespace StreamJsonRpc;
 [RequiresDynamicCode(RuntimeReasons.RefEmit), RequiresUnreferencedCode(RuntimeReasons.RefEmit)]
 internal static class ProxyGeneration
 {
+#if NET
+    private static readonly List<(AssemblyLoadContext, ImmutableHashSet<AssemblyName> SkipVisibilitySet, ModuleBuilder Builder)> TransparentProxyModuleBuilderByVisibilityCheck = [];
+#else
     private static readonly List<(ImmutableHashSet<AssemblyName> SkipVisibilitySet, ModuleBuilder Builder)> TransparentProxyModuleBuilderByVisibilityCheck = new List<(ImmutableHashSet<AssemblyName>, ModuleBuilder)>();
+#endif
     private static readonly object BuilderLock = new object();
     private static readonly AssemblyName ProxyAssemblyName = new AssemblyName(string.Format(CultureInfo.InvariantCulture, "StreamJsonRpc_Proxies_{0}", Guid.NewGuid()));
     private static readonly MethodInfo DelegateCombineMethod = typeof(Delegate).GetRuntimeMethod(nameof(Delegate.Combine), new Type[] { typeof(Delegate), typeof(Delegate) })!;
@@ -110,6 +117,9 @@ internal static TypeInfo Get(Type contractInterface, ReadOnlySpan<Type> addition
             // Rpc interfaces must be sorted so that we implement methods from base interfaces before those from their derivations.
             SortRpcInterfaces(rpcInterfaces);
 
+            // For ALC selection reasons, it's vital that the *user's* selected interfaces come *before* our own supporting interfaces.
+            // If the order is incorrect, type resolution may fail or the wrong AssemblyLoadContext (ALC) may be selected,
+            // leading to runtime errors or unexpected behavior when loading types or invoking methods.
             Type[] proxyInterfaces = [.. rpcInterfaces.Select(i => i.Type), typeof(IJsonRpcClientProxy), typeof(IJsonRpcClientProxyInternal)];
             ModuleBuilder proxyModuleBuilder = GetProxyModuleBuilder(proxyInterfaces);
 
@@ -783,10 +793,27 @@ private static ModuleBuilder GetProxyModuleBuilder(Type[] interfaceTypes)
         // For each set of skip visibility check assemblies, we need a dynamic assembly that skips at *least* that set.
         // The CLR will not honor any additions to that set once the first generated type is closed.
         // We maintain a dictionary to point at dynamic modules based on the set of skip visibility check assemblies they were generated with.
-        ImmutableHashSet<AssemblyName> skipVisibilityCheckAssemblies = ImmutableHashSet.CreateRange(interfaceTypes.SelectMany(t => SkipClrVisibilityChecks.GetSkipVisibilityChecksRequirements(t.GetTypeInfo())))
+        ImmutableHashSet<AssemblyName> skipVisibilityCheckAssemblies = ImmutableHashSet.CreateRange(AssemblyNameEqualityComparer.Instance, interfaceTypes.SelectMany(t => SkipClrVisibilityChecks.GetSkipVisibilityChecksRequirements(t.GetTypeInfo())))
             .Add(typeof(ProxyGeneration).Assembly.GetName());
+#if NET
+        // We have to key the dynamic assembly by ALC as well, since callers may set a custom contextual reflection context
+        // that influences how the assembly will resolve its type references.
+        // If they haven't set a contextual one, we assume the ALC that defines the (first) proxy interface.
+        AssemblyLoadContext alc = AssemblyLoadContext.CurrentContextualReflectionContext
+            ?? AssemblyLoadContext.GetLoadContext(interfaceTypes[0].Assembly)
+            ?? AssemblyLoadContext.GetLoadContext(typeof(ProxyGeneration).Assembly)
+            ?? throw new Exception("No ALC for our own assembly!");
+        foreach ((AssemblyLoadContext AssemblyLoadContext, ImmutableHashSet<AssemblyName> SkipVisibilitySet, ModuleBuilder Builder) existingSet in TransparentProxyModuleBuilderByVisibilityCheck)
+        {
+            if (existingSet.AssemblyLoadContext != alc)
+            {
+                continue;
+            }
+
+#else
         foreach ((ImmutableHashSet<AssemblyName> SkipVisibilitySet, ModuleBuilder Builder) existingSet in TransparentProxyModuleBuilderByVisibilityCheck)
         {
+#endif
             if (existingSet.SkipVisibilitySet.IsSupersetOf(skipVisibilityCheckAssemblies))
             {
                 return existingSet.Builder;
@@ -798,11 +825,22 @@ private static ModuleBuilder GetProxyModuleBuilder(Type[] interfaceTypes)
         // I have disabled this optimization though till we need it since it would sometimes cover up any bugs in the above visibility checking code.
         ////skipVisibilityCheckAssemblies = skipVisibilityCheckAssemblies.Union(AppDomain.CurrentDomain.GetAssemblies().Select(a => a.GetName()));
 
-        AssemblyBuilder assemblyBuilder = CreateProxyAssemblyBuilder();
+        AssemblyBuilder assemblyBuilder;
+#if NET
+        using (alc.EnterContextualReflection())
+#endif
+        {
+            assemblyBuilder = CreateProxyAssemblyBuilder();
+        }
+
         ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("rpcProxies");
         var skipClrVisibilityChecks = new SkipClrVisibilityChecks(assemblyBuilder, moduleBuilder);
         skipClrVisibilityChecks.SkipVisibilityChecksFor(skipVisibilityCheckAssemblies);
+#if NET
+        TransparentProxyModuleBuilderByVisibilityCheck.Add((alc, skipVisibilityCheckAssemblies, moduleBuilder));
+#else
         TransparentProxyModuleBuilderByVisibilityCheck.Add((skipVisibilityCheckAssemblies, moduleBuilder));
+#endif
 
         return moduleBuilder;
     }

src/StreamJsonRpc/SkipClrVisibilityChecks.cs

@@ -82,7 +82,7 @@ internal static ImmutableHashSet<AssemblyName> GetSkipVisibilityChecksRequiremen
         Requires.NotNull(typeInfo, nameof(typeInfo));
 
         var visitedTypes = new HashSet<TypeInfo>();
-        ImmutableHashSet<AssemblyName>.Builder assembliesDeclaringInternalTypes = ImmutableHashSet.CreateBuilder<AssemblyName>(AssemblyNameEqualityComparer.Instance);
+        ImmutableHashSet<AssemblyName>.Builder assembliesDeclaringInternalTypes = ImmutableHashSet.CreateBuilder(AssemblyNameEqualityComparer.Instance);
         CheckForNonPublicTypes(typeInfo, assembliesDeclaringInternalTypes, visitedTypes);
 
         // Enumerate members on the interface that we're going to need to implement.
@@ -256,35 +256,4 @@ private TypeInfo EmitMagicAttribute()
 
         return tb.CreateTypeInfo()!;
     }
-
-    private class AssemblyNameEqualityComparer : IEqualityComparer<AssemblyName>
-    {
-        internal static readonly IEqualityComparer<AssemblyName> Instance = new AssemblyNameEqualityComparer();
-
-        private AssemblyNameEqualityComparer()
-        {
-        }
-
-        public bool Equals(AssemblyName? x, AssemblyName? y)
-        {
-            if (x is null && y is null)
-            {
-                return true;
-            }
-
-            if (x is null || y is null)
-            {
-                return false;
-            }
-
-            return string.Equals(x.FullName, y.FullName, StringComparison.OrdinalIgnoreCase);
-        }
-
-        public int GetHashCode(AssemblyName obj)
-        {
-            Requires.NotNull(obj, nameof(obj));
-
-            return StringComparer.OrdinalIgnoreCase.GetHashCode(obj.FullName);
-        }
-    }
 }

test/StreamJsonRpc.Tests.NoInterceptors/StreamJsonRpc.Tests.NoInterceptors.csproj

@@ -15,6 +15,7 @@
 
   <ItemGroup>
     <Compile Include="..\StreamJsonRpc.Tests\JsonRpcProxyGenerationTests.cs" Link="JsonRpcProxyGenerationTests.cs" />
+    <Compile Include="..\StreamJsonRpc.Tests\UnreachableAssemblyTools.cs" Link="UnreachableAssemblyTools.cs" />
     <Compile Include="..\StreamJsonRpc.Tests\Usings.cs" Link="Usings.cs" />
   </ItemGroup>
   <ItemGroup>
@@ -41,6 +42,7 @@
   <ItemGroup>
     <Reference Include="Microsoft.CSharp" Condition=" '$(TargetFramework)' == 'net472' " />
   </ItemGroup>
+  <Import Project="..\UnreachableAssembly.targets" />
   <Import Project="$(RepoRootPath)src\AnalyzerUser.targets" />
   <PropertyGroup>
     <EnableStreamJsonRpcInterceptors>false</EnableStreamJsonRpcInterceptors>

test/StreamJsonRpc.Tests/JsonRpcProxyGenerationTests.cs

@@ -5,9 +5,13 @@
 
 using System.Diagnostics;
 using System.Reflection;
+#if NET
+using System.Runtime.Loader;
+#endif
 using Microsoft.VisualStudio.Threading;
 using Nerdbank;
 using StreamJsonRpc.Reflection;
+using StreamJsonRpc.Tests;
 using ExAssembly = StreamJsonRpc.Tests.ExternalAssembly;
 
 public abstract partial class JsonRpcProxyGenerationTests : TestBase
@@ -162,6 +166,12 @@ public interface IServerWithGenericMethod
         Task AddAsync<T>(T a, T b);
     }
 
+    [JsonRpcContract]
+    public partial interface IReferenceAnUnreachableAssembly
+    {
+        Task TakeAsync(UnreachableAssembly.SomeUnreachableClass obj);
+    }
+
     [JsonRpcContract]
     internal partial interface IServerInternal :
         ExAssembly.ISomeInternalProxyInterface,
@@ -880,6 +890,44 @@ public async Task ValueTaskReturningMethod()
         await clientRpc.DoSomethingValueAsync();
     }
 
+    /// <summary>
+    /// Validates that similar proxies are generated in the same dynamic assembly.
+    /// </summary>
+    [Fact]
+    public void ReuseDynamicAssembliesTest()
+    {
+        JsonRpc clientRpc = new(Stream.Null);
+        IServer proxy1 = clientRpc.Attach<IServer>(this.DefaultProxyOptions);
+        IServer2 proxy2 = clientRpc.Attach<IServer2>(this.DefaultProxyOptions);
+        Assert.Same(proxy1.GetType().Assembly, proxy2.GetType().Assembly);
+    }
+
+#if NET
+    [Fact]
+    public void DynamicAssembliesKeyedByAssemblyLoadContext()
+    {
+        UnreachableAssemblyTools.VerifyUnreachableAssembly();
+
+        // Set up a new ALC that can find the hidden assembly, and ask for the proxy type.
+        AssemblyLoadContext alc = UnreachableAssemblyTools.CreateContextForReachingTheUnreachable();
+
+        JsonRpc clientRpc = new(Stream.Null);
+
+        // Ensure we first generate a proxy in our own default ALC.
+        // The goal being to emit a DynamicAssembly that we *might* reuse
+        // for the later proxy for which the first DynamicAssembly is not appropriate.
+        clientRpc.Attach<IServer>(this.DefaultProxyOptions);
+
+        // Now take very specific steps to invoke the rest of the test in the other AssemblyLoadContext.
+        // This is important so that our IReferenceAnUnreachableAssembly type will be able to resolve its
+        // own type references to UnreachableAssembly.dll, which our own default ALC cannot do.
+        MethodInfo helperMethodInfo = typeof(JsonRpcProxyGenerationTests).GetMethod(nameof(DynamicAssembliesKeyedByAssemblyLoadContext_Helper), BindingFlags.NonPublic | BindingFlags.Static)!;
+        MethodInfo helperWithinAlc = UnreachableAssemblyTools.LoadHelperInAlc(alc, helperMethodInfo);
+        helperWithinAlc.Invoke(null, [this.DefaultProxyOptions]);
+    }
+
+#endif
+
     protected T AttachJsonRpc<T>(Stream stream)
         where T : class
     {
@@ -889,6 +937,23 @@ protected T AttachJsonRpc<T>(Stream stream)
         return proxy;
     }
 
+#if NET
+
+    private static void DynamicAssembliesKeyedByAssemblyLoadContext_Helper(JsonRpcProxyOptions options)
+    {
+        // Although this method executes within the special ALC,
+        // StreamJsonRpc is loaded in the default ALC.
+        // Therefore unless StreamJsonRpc is taking care to use a DynamicAssembly
+        // that belongs to *this* ALC, it won't be able to resolve the same type references
+        // that we can here (the ones from UnreachableAssembly).
+        // That's what makes this test effective: it'll fail if the DynamicAssembly is shared across ALCs,
+        // thereby verifying that StreamJsonRpc has a dedicated set of DynamicAssemblies for each ALC.
+        JsonRpc clientRpc = new(Stream.Null);
+        clientRpc.Attach<IReferenceAnUnreachableAssembly>(options);
+    }
+
+#endif
+
 #if NO_INTERCEPTORS
     public class Dynamic(ITestOutputHelper logger) : JsonRpcProxyGenerationTests(logger, JsonRpcProxyOptions.ProxyImplementation.AlwaysDynamic);
 #else

test/StreamJsonRpc.Tests/StreamJsonRpc.Tests.csproj

@@ -81,5 +81,6 @@
   <ItemGroup>
     <Reference Include="Microsoft.CSharp" Condition=" '$(TargetFramework)' == 'net472' " />
   </ItemGroup>
+  <Import Project="..\UnreachableAssembly.targets" />
   <Import Project="$(RepoRootPath)src\AnalyzerUser.targets" />
 </Project>

test/StreamJsonRpc.Tests/UnreachableAssemblyTools.cs

@@ -0,0 +1,50 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#if NET
+
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.Loader;
+
+namespace StreamJsonRpc.Tests;
+
+internal static class UnreachableAssemblyTools
+{
+    /// <summary>
+    /// Useful for tests to call before asserting conditions that depend on the UnreachableAssembly.dll
+    /// actually being unreachable.
+    /// </summary>
+    internal static void VerifyUnreachableAssembly()
+    {
+        Assert.Throws<FileNotFoundException>(() => typeof(UnreachableAssembly.SomeUnreachableClass));
+    }
+
+    /// <summary>
+    /// Initializes an <see cref="AssemblyLoadContext"/> with UnreachableAssembly.dll loaded into it.
+    /// </summary>
+    /// <param name="testName">The name to give the <see cref="AssemblyLoadContext"/>.</param>
+    /// <returns>The new <see cref="AssemblyLoadContext"/>.</returns>
+    internal static AssemblyLoadContext CreateContextForReachingTheUnreachable([CallerMemberName] string? testName = null)
+    {
+        AssemblyLoadContext alc = new(testName);
+        alc.LoadFromAssemblyPath(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!, "hidden", "UnreachableAssembly.dll"));
+        return alc;
+    }
+
+    /// <summary>
+    /// Translates a <see cref="MethodInfo"/> from one ALC into another ALC, so that it can be invoked
+    /// within the context of the new ALC.
+    /// </summary>
+    /// <param name="alc">The <see cref="AssemblyLoadContext"/> to load the method into.</param>
+    /// <param name="helperMethodInfo">The <see cref="MethodInfo"/> of the method in the caller's ALC to load into the given <paramref name="alc"/>.</param>
+    /// <returns>The translated <see cref="MethodInfo"/>.</returns>
+    internal static MethodInfo LoadHelperInAlc(AssemblyLoadContext alc, MethodInfo helperMethodInfo)
+    {
+        Assembly selfWithinAlc = alc.LoadFromAssemblyPath(helperMethodInfo.DeclaringType!.Assembly.Location);
+        MethodInfo helperWithinAlc = (MethodInfo)selfWithinAlc.ManifestModule.ResolveMethod(helperMethodInfo.MetadataToken)!;
+        return helperWithinAlc;
+    }
+}
+
+#endif

test/UnreachableAssembly.targets

@@ -0,0 +1,14 @@
+<Project>
+  <ItemGroup>
+    <ProjectReference Include="..\UnreachableAssembly\UnreachableAssembly.csproj">
+      <!-- We MUST NOT have this assembly in our test assembly's output directory so that it cannot be reached from the default ALC. -->
+      <Private>false</Private>
+    </ProjectReference>
+  </ItemGroup>
+  <Target Name="PlaceMissingAssembly" AfterTargets="ResolveReferences">
+    <ItemGroup>
+      <DivertedProjectReferenceOutputs Include="@(ReferencePath)" Condition=" '%(FileName)' == 'UnreachableAssembly' " />
+    </ItemGroup>
+    <Copy SourceFiles="@(DivertedProjectReferenceOutputs)" DestinationFolder="$(OutputPath)hidden" SkipUnchangedFiles="true" />
+  </Target>
+</Project>