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.

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

CreateInterfaceProxyWithoutTarget throws NotImplemented when interface has method with generic argument in Blazor WebAssembly #585

Closed
teneko opened this issue Mar 14, 2021 · 9 comments

Comments

@teneko
Copy link

teneko commented Mar 14, 2021

A internal call of Castle.DynamicProxy.Internal.TypeUtil.GetClosedParameterType(AbstractTypeEmitter type, Type parameter) seems to produce a NotImplementedException when it is calling System.RuntimeType.MakeGenericType(Type[] instantiation) during proxy creation.

This bug is only reproducable in Blazor WebAssembly and therefore with the Mono Runtime (!?).

The relevant code:

public interface MonoProblematicInterface
{
    // THIS IS IRONY.
    public void DiscriminatedFromMonoBecauseImGeneric<T>();
}

var proxy = new DynamicProxy.ProxyGenerator().CreateInterfaceProxyWithoutTarget(typeof(MonoProblematicInterface));

Stacktrace:

crit: Microsoft.AspNetCore.Components.WebAssembly.Rendering.WebAssemblyRenderer[100]
      Unhandled exception rendering component: The method or operation is not implemented.
System.NotImplementedException: The method or operation is not implemented.
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at Castle.DynamicProxy.Generators.MethodWithInvocationGenerator.BuildProxiedMethodBody(MethodEmitter emitter, ClassEmitter class, ProxyGenerationOptions options, INamingScope namingScope)
   at Castle.DynamicProxy.Generators.MethodGenerator.Generate(ClassEmitter class, ProxyGenerationOptions options, INamingScope namingScope)
   at Castle.DynamicProxy.Contributors.CompositeTypeContributor.ImplementMethod(MetaMethod method, ClassEmitter class, ProxyGenerationOptions options, OverrideMethodDelegate overrideMethod)
   at Castle.DynamicProxy.Contributors.CompositeTypeContributor.Generate(ClassEmitter class, ProxyGenerationOptions options)
   at Castle.DynamicProxy.Generators.InterfaceProxyWithoutTargetGenerator.GenerateType(String typeName, Type proxyTargetType, Type[] interfaces, INamingScope namingScope)
   at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.<>c__DisplayClass6_0.<GenerateCode>b__0(String n, INamingScope s)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.<>c__DisplayClass33_0.<ObtainProxyType>b__0(CacheKey _)
   at Castle.Core.Internal.SynchronizedDictionary`2[[Castle.DynamicProxy.Generators.CacheKey, Castle.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc],[System.Type, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetOrAdd(CacheKey key, Func`2 valueFactory)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 factory)
   at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.GenerateCode(Type proxyTargetType, Type[] interfaces, ProxyGenerationOptions options)
   at Castle.DynamicProxy.DefaultProxyBuilder.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
   at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
   at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, IInterceptor[] interceptors)
   at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, IInterceptor[] interceptors)
   at Castle.Core.WebAssembly.Bug.Pages.Index.OnAfterRender(Boolean firstRender) in C:\Users\<user>\source\repos\Castle.Core.WebAssembly.Bug\Castle.Core.WebAssembly.Bug\Pages\Index.razor:line 102
   at Microsoft.AspNetCore.Components.ComponentBase.Microsoft.AspNetCore.Components.IHandleAfterRender.OnAfterRenderAsync()
   at Microsoft.AspNetCore.Components.Rendering.ComponentState.NotifyRenderCompletedAsync()

Here a small project with the feasible code: https://github.com/teroneko/Castle.Core.WebAssembly.Bug

@teneko
Copy link
Author

teneko commented Mar 14, 2021

It just triggers me why it works in .NET 5, but not in Mono. Maybe you can explain to me, @stakx?

@stakx
Copy link
Member

stakx commented Mar 21, 2021

@teroneko, first a disclaimer: I don't have any practical experience with Blazor, so at this time, you'll only get semi-educated guesses from me, but nothing definite.

First, I cannot confirm your claim that "it works in .NET 5"; your sample project above targets .net5.0, and the error does occur, too. So as far as I currently understand it, there's no reason to assume that this issue is specific to the Mono runtime. (But I may be simplifying here, now that Mono is under the .NET umbrella and just one of several runtimes that can be chosen, perhaps net5.0 doesn't exclude Mono per se?)

I am suspecting that this really boils down to Blazor making use of ahead-of-time (AOT) compilation when translating to WASM. AOT is not a scenario that DynamicProxy was designed for. DynamicProxy uses System.Reflection.Emit to generate types at runtime, and this doesn't work on several mobile platforms; either due to AOT & the target platform simply not supporting runtime code generation; or restrictions placed on dynamic code generation at runtime (usually for reasons of security).

Whether that is also the case for Blazor / WebAssembly, I cannot say without doing some further reading. Maybe someone else can chime in and share their knowledge or experiences?

@teneko
Copy link
Author

teneko commented Mar 21, 2021

First, I cannot confirm your claim that "it works in .NET 5"; your sample project above targets .net5.0, and the error does occur, too

You are assume that net5.0 is equivalent to runtime? That's not the case; the API surface is called net5.0 too. It just matters at which runtime the API is called at: net5.0 or mono. Here an excerpt from Sterven Anderson's Blog:

http://blog.stevensanderson.com/2017/11/05/blazor-on-mono/:

Blazor on Mono

Based on Rodrigo’s excellent work, I’ve migrated Blazor to run on Mono. The result is that it has the same features as before, >minus the prototype debugging support, but now it executes faster and it supports a vastly more complete .NET API surface, [..]

My claim I have just made because my xUnit tests which involves generic constrained methods in interface (that get's proxyfied) and run on .net 5 runtime all pass, but my e2e tests which involveds generic constrained methods in interface all fail on mono runtime. Thus the reproducable repo example.

I have added an xUnit project where the test succeed with same interface and proxy creation procedure like in the WebAssembly project which should underline my claim. 🙂

Like I told, your pull request "Match generic parameters by position, not by name" fixes the problem. In the meantime I have forked your project and created a hopefully short-lived package that I happily would unlist. 😄

@stakx
Copy link
Member

stakx commented Mar 22, 2021

@teroneko thanks for the clarification re: net5.0 and Mono, I suspected I may be missing something there, but wasn't certain.

Great to hear that my unrelated PR solves your problem, even though I suspect there may be some good fortune involved in this. I still think the likelier root cause is that Blazor / WASM doesn't have full support for System.Reflection.Emit – if so, you may run into other problems when using DP in a Blazor project. (This isn't a platform that we're running our test suite on.)

@teneko
Copy link
Author

teneko commented Mar 22, 2021

It is not that dramatic. Regarding this dotnet/runtime#37794 it is about to be tackled?

As I am writing a library for working better with Mirosoft.JSInterop in Blazor I am facing no problems so far. I use your created proxy without target to forward manually the invocation to target and all other invocations are transformed to in-built methods of target.

What surprises me is, you test against Mono? I mean you are testing on Ubuntu and .NET Framework. Shouldn't the problem arise too like in the repo?

I would likely to hear whether you are accepting your own pull request in the near future, as it seems to solve this. I think you are awaiting review, isn't it?

@stakx
Copy link
Member

stakx commented Mar 22, 2021

Mono and WebAssembly are two distinct technologies. From what I currently understand, Blazor makes use of a Mono runtime that has been AOT-compiled to WebAssembly. OTOH, we're just running our test suite on a regular, standalone Mono runtime that JIT-compiles IL bytecode down to x86/x64, which is a rather different setup. I'll stress once again that the likely culprit is AOT compilation (which may play a role with Blazor, but doesn't happen during our regular test runs).

I would likely to hear whether you are accepting your own pull request in the near future

Yes, I've kept it open so it can be reviewed, but I might eventually go ahead and merge it.

Releases are done by @jonorossi so he could tell you when the next one will be published – most of the work that'll likely go into it has been finished or is close to being done, we're still trying to wrap up a few smaller things here and there AFAIK.

@jonorossi
Copy link
Member

@stakx I'll take a look at the pull request tomorrow, too late tonight. Sorry for being so slack getting to these.

@teneko
Copy link
Author

teneko commented Mar 22, 2021

@stakx Thanks for stressing it up. I read about AOT, that this is the way to get the WebAssembly feature to work but didn't got the picture in relation to the the problem of this issue. Now most adds up!

I am just grateful for this project - don't want to rush you. 🙂

@stakx
Copy link
Member

stakx commented Apr 21, 2021

Given that the immediate problem described in this issue will be solved by the PR now merged, and given that we don't really have direct support for platforms missing JIT code generation via Reflection.Emit, I think we can close this issue. If noone says otherwise, I'll close in a few days' time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants