Skip to content

Commit 5461a74

Browse files
committed
Create dynamic proxies in fewer dynamic assemblies
This corrects a bug that deviated from the original design. The idea was for all proxies to be generated into as few dynamic assemblies as possible. We have to allow for distinct dynamic assemblies for growing sets of skip visibility check attributes, but other than that we should reuse dynamic assemblies. The bug here was that we were not supplying a structural `AssemblyName` equality comparer to our `ImmutableHashSet`. Since `AssemblyName.Equals` is a reference equality check and the CLR does not de-dupe assembly names, we must supply our own equality comparison in order to get the intended behavior.
1 parent 7d3021c commit 5461a74

File tree

4 files changed

+50
-33
lines changed

4 files changed

+50
-33
lines changed
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
3+
using System.Reflection;
4+
5+
namespace StreamJsonRpc;
6+
7+
internal class AssemblyNameEqualityComparer : IEqualityComparer<AssemblyName>
8+
{
9+
internal static readonly IEqualityComparer<AssemblyName> Instance = new AssemblyNameEqualityComparer();
10+
11+
private AssemblyNameEqualityComparer()
12+
{
13+
}
14+
15+
public bool Equals(AssemblyName? x, AssemblyName? y)
16+
{
17+
if (x is null && y is null)
18+
{
19+
return true;
20+
}
21+
22+
if (x is null || y is null)
23+
{
24+
return false;
25+
}
26+
27+
return string.Equals(x.FullName, y.FullName, StringComparison.OrdinalIgnoreCase);
28+
}
29+
30+
public int GetHashCode(AssemblyName obj)
31+
{
32+
Requires.NotNull(obj, nameof(obj));
33+
34+
return StringComparer.OrdinalIgnoreCase.GetHashCode(obj.FullName);
35+
}
36+
}

src/StreamJsonRpc/ProxyGeneration.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ private static ModuleBuilder GetProxyModuleBuilder(Type[] interfaceTypes)
752752
// For each set of skip visibility check assemblies, we need a dynamic assembly that skips at *least* that set.
753753
// The CLR will not honor any additions to that set once the first generated type is closed.
754754
// We maintain a dictionary to point at dynamic modules based on the set of skip visibility check assemblies they were generated with.
755-
ImmutableHashSet<AssemblyName> skipVisibilityCheckAssemblies = ImmutableHashSet.CreateRange(interfaceTypes.SelectMany(t => SkipClrVisibilityChecks.GetSkipVisibilityChecksRequirements(t.GetTypeInfo())))
755+
ImmutableHashSet<AssemblyName> skipVisibilityCheckAssemblies = ImmutableHashSet.CreateRange(AssemblyNameEqualityComparer.Instance, interfaceTypes.SelectMany(t => SkipClrVisibilityChecks.GetSkipVisibilityChecksRequirements(t.GetTypeInfo())))
756756
.Add(typeof(ProxyGeneration).Assembly.GetName());
757757
foreach ((ImmutableHashSet<AssemblyName> SkipVisibilitySet, ModuleBuilder Builder) existingSet in TransparentProxyModuleBuilderByVisibilityCheck)
758758
{

src/StreamJsonRpc/SkipClrVisibilityChecks.cs

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ internal static ImmutableHashSet<AssemblyName> GetSkipVisibilityChecksRequiremen
7979
Requires.NotNull(typeInfo, nameof(typeInfo));
8080

8181
var visitedTypes = new HashSet<TypeInfo>();
82-
ImmutableHashSet<AssemblyName>.Builder assembliesDeclaringInternalTypes = ImmutableHashSet.CreateBuilder<AssemblyName>(AssemblyNameEqualityComparer.Instance);
82+
ImmutableHashSet<AssemblyName>.Builder assembliesDeclaringInternalTypes = ImmutableHashSet.CreateBuilder(AssemblyNameEqualityComparer.Instance);
8383
CheckForNonPublicTypes(typeInfo, assembliesDeclaringInternalTypes, visitedTypes);
8484

8585
// Enumerate members on the interface that we're going to need to implement.
@@ -253,35 +253,4 @@ private TypeInfo EmitMagicAttribute()
253253

254254
return tb.CreateTypeInfo()!;
255255
}
256-
257-
private class AssemblyNameEqualityComparer : IEqualityComparer<AssemblyName>
258-
{
259-
internal static readonly IEqualityComparer<AssemblyName> Instance = new AssemblyNameEqualityComparer();
260-
261-
private AssemblyNameEqualityComparer()
262-
{
263-
}
264-
265-
public bool Equals(AssemblyName? x, AssemblyName? y)
266-
{
267-
if (x is null && y is null)
268-
{
269-
return true;
270-
}
271-
272-
if (x is null || y is null)
273-
{
274-
return false;
275-
}
276-
277-
return string.Equals(x.FullName, y.FullName, StringComparison.OrdinalIgnoreCase);
278-
}
279-
280-
public int GetHashCode(AssemblyName obj)
281-
{
282-
Requires.NotNull(obj, nameof(obj));
283-
284-
return StringComparer.OrdinalIgnoreCase.GetHashCode(obj.FullName);
285-
}
286-
}
287256
}

test/StreamJsonRpc.Tests/JsonRpcProxyGenerationTests.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,18 @@ public async Task ValueTaskReturningMethod()
798798
await clientRpc.DoSomethingValueAsync();
799799
}
800800

801+
/// <summary>
802+
/// Validates that similar proxies are generated in the same dynamic assembly.
803+
/// </summary>
804+
[Fact]
805+
public void ReuseDynamicAssembliesTest()
806+
{
807+
JsonRpc clientRpc = new(Stream.Null);
808+
IServer proxy1 = clientRpc.Attach<IServer>();
809+
IServer2 proxy2 = clientRpc.Attach<IServer2>();
810+
Assert.Same(proxy1.GetType().Assembly, proxy2.GetType().Assembly);
811+
}
812+
801813
public class EmptyClass
802814
{
803815
}

0 commit comments

Comments
 (0)