dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.46k stars 4.76k forks source link

DispatchProxy generates a proxy type implementing the interface from the wrong ALC #109398

Open mconnew opened 4 weeks ago

mconnew commented 4 weeks ago

Description

When an assembly loaded into a custom AssemblyLoadContext creates a proxy using DispatchProxy for an interface from an assembly loaded into that AssemblyLoadContext, the first custom ALC works fine. If you create a second custom AssemblyLoadContext and load the same assembly and try to create a proxy for the same interface, but for the interface type loaded into the second ALC, the returned proxy incorrectly implements the interface from the first ALC.
This is an issue when the DispatchProxy proxyType is from the Default ALC (or at least not the same ALC as the interface being proxied).

Reproduction Steps

I created a solution here which demonstrates the problem:
https://github.com/mconnew/IssueRepros/tree/main/DispatchProxyWithAlc

Expected behavior

The second custom ALC instance should create the proxy implementing the interface loaded into the second ALC instance.

Actual behavior

The second custom ALC instance creates the proxy implementing the interface that's loaded into the first ALC instance.

Regression?

No response

Known Workarounds

Explicitly load the assembly which contains the proxyType into the ALC that contains the interface being proxied. The repro app purposefully puts the proxyType implementation into a third assembly to mimic the typical WCF scenario where the WCF client libraries are usually used from the default ALC and not explicitly loaded.

Configuration

.NET 8 Windows 11, x64 Not specific to configuration, this had been reported by multiple users.

Other information

This is affecting WCF scenarios. This issue has been reported occurring with Azure Functions (via Microsoft support) and PowerPlatform Dataverse. Here's the Dataverse issue with it reported. Someone mentions about seeing the issue in Azure Functions at the end of this issue.

Here's an issue where it was previously reported, and I don't believe the problem was fully understood and incorrectly closed as by design. Returning a proxy for an interface type different than the one you asked for (as the same interface in different ALC's at runtime are treated as different types) isn't the design of this feature.

mconnew commented 4 weeks ago

I don't think this bug exists in DispatchProxy, I believe it's in the Ref.Emit implementation.

dotnet-policy-service[bot] commented 4 weeks ago

Tagging subscribers to this area: @dotnet/area-system-reflection-emit See info in area-owners.md if you want to be subscribed.

jkotas commented 4 weeks ago

Likely the same root cause as #101343

mconnew commented 4 weeks ago

@jkotas, I think I found the root cause of the problem. DispatchProxyGenerator looks up the ALC of the base proxyType and uses that as the key in s_alcProxyAssemblyMap which maps the ALC to an instance of ProxyAssembly. The ProxyAssembly class creates a persistent AssemblyBuilder and ModuleBuilder to hold generated types. When creating the first custom ALC, and the first generated proxy, because the base proxyType is from an assembly loaded into the default ALC, DispatchProxyGenerator creates a ProxyAssembly instance mapped to the default ALC. When creating the proxy, it references the proxied interface (due to implementing it), so the runtime interface type is resolved and kept track in the generated Assembly/AssemblyBuilder. When creating a proxy in the second ALC, as the base proxyType is in the default ALC, the same ProxyAssembly instance is used, which means the second generated proxy is created using the same AssemblyBuilder/ModuleBuilder as the first one. The process of adding implemented interfaces to the generated type does so using the types integer token value that's looked up using some QCalls. As the AssemblyBuilder already references that interface token value, there's no need to go to all the trouble to resolve it, that's already been done. So when the proxy is generated and an instance created, the type implemented is the one that AssemblyBuilder has already resolved, which is the instance from the first ALC. To reference the interface in a second ALC, a new ProxyAssembly instance would need to be used.

So this is a DispatchProxy bug. Basically the key for s_alcProxyAssemblyMap is incorrect. It should be a two-tuple of the proxyType ALC and the proxied interface type ALC. This way if the base proxyType is in the default ALC, but the interface type is in a custom ALC, you can have multiple custom ALC instances each proxying the same interface type and not end up with a cast problem as the proxies for the separate ALC's will be created by separate ProxyAssembly instances, which means separate AssemblyBuilder instances, so you won't cross contaminate type references. Just switching to keying off the interface ALC type won't be sufficient as you could have a single interface type instance from a single ALC that is used to create a DispatchProxy using the same base proxyType, but the proxyType Type instance could be for different ALC instances. You could have the following situation:

Default ALC
DPI.dll - DispatchProxyUtil Logging.dll - LoggingExtensions.WrapInterface\<T>(T instance)

ALC1 Foo.dll - IFace1 DPI.dll - DispatchProxyUtil

Foo.dll has some code which calls LoggingExtensions.WrapInterface to wrap calls to interface IFace1 with some logging. The code in Logging.dll uses DispatchProxy specifying DispatchProxyUtil as the base proxyType and uses the interface specified. DispatchProxyUtil has extensibility points which enables being able to run some code before and after proxying the call to the wrapped instance, which in this case is used to log how long the method took. The generated proxy should have proxyType ALC = Default and interface ALC = ALC1. The problem though is that the instance passed by Foo.dll to LoggingExtensions.WrapInterface is already a generated proxy created by some other code in Foo.dll using DispatchProxyUtil as the base proxyType. As ALC1 has DPI.dll loaded, this first proxy is created with proxyType ALC = ALC1 and interface ALC = ALC1. So when LoggingExtensions.WrapInterface takes the instance returned by DispatchProxy.Create and tries to cast it to DispatchProxyUtil to set up the logging behavior before returning it, if ProxyAssembly is only keyed off the interface ALC, the cast fails. This is because the ProxyAssembly instance for ALC1 will generate a proxy using the DispatchProxyUtil type loaded in ALC1, and when LoggingExtensions.WrapInterface tries to create a proxy, as it's the same interface from the same ALC, it would use the same ProxyAssembly instance which would use the DispatchProxyUtil type instance from ALC1. Then when WrapInterface tries to cast the returned implementation to DispatchProxyUtil, you'll get an invalid cast exception.

This way around is a lot more convoluted to come up with a breaking scenario than using the proxyType ALC as the key, so it might be okay to switch the set of broken scenarios to the smaller less likely set. I do believe we should use the full two-tuple though. That's going to be non-trivial to do though due to use of ConditionalWeakTable. You could do a double layered setup where s_alcProxyAssemblyMap is defined as ConditionalWeakTable<AssemblyLoadContext, ConditionalWeakTable<AssemblyLoadContext, ProxyAssembly>>. You just nest the lookup by each ALC one at a time. I don't think proxy creation itself should be considered a hot code path. You could also optimize things. Have a single static ProxyAssembly for the most common case that both types are the default ALC. Have a separate dictionary for the common case of neither type being in a collectible ALC (Dictionary<Tuple<AssemblyLoadContext,AssemblyLoadContext>, ProxyAssembly>), only falling back to the two layered ConditionalWeakTable if either is collectible.

The repro app I created also demonstrates another problem. If the base proxyType comes from the default ALC, you can't have the proxied interface type exist in a collectible ALC. You get an exception as trying to do so would pin the collectible ALC by having a type generated in the default ALC that references (by way of implementing it) an interface type from the collectible ALC. The utility method AssemblyLoadContext.EnterContextualReflection should be used to enable this scenario. If the interface type is in a collectible ALC, a call should be made to EnterContextualReflection to use the interface types ALC before instantiating the ProxyAssembly (presuming AssemblyBuilder/ModuleBuilder capture the ALC on creation, otherwise might need to do that on first proxy generation).

steveharter commented 3 weeks ago

Verified this issue existed back to v7. Moving to future based on assumed lower priority.

@mconnew are you planning to continue to look at this and perhaps provide a fix for v10? Thanks for the issue, repro and research here.