[WIP][CoreCLR] Fix Delegate marshalling to function pointers #10493
Conversation
|
@simonrozsival we could parameterize public void MonoAndroidExportReferencedAppStarts (bool embedAssemblies, bool isRelease, AndroidRuntime runtime)
// later
proj.SetRuntime (runtime);And just add a case for |
| return_type_info.NativeType, paramTypes.ToArray ()); | ||
| m.Generate (dm.GetILGenerator ()); | ||
| result = dm.CreateDelegate (GetDelegateType ()); | ||
| result = tb.CreateType ().GetMethod (name).MethodHandle.GetFunctionPointer (); |
There was a problem hiding this comment.
This change appears to be why dotnet/java-interop#1364 is needed.
Is there no other way to do this, though?
(Arguably yes, what's done here is more efficient, but I'm reluctant to "like" it because of the API and ABI break that dotnet/java-interop#1364 requires.)
"Instead", you can use TypeBuilder to create a new MulticastDelegate subclass, and then use Delegate.CreateDelegate() around the newly created Delegate + tb.CreateType().GetMethod(name).
"Even better", we have (unshipped) code which creates such delegate types!
As far as I can tell, the only "real" reason for dotnet/java-interop#1364 is efficiency, which, fair. However, it's quite the API and ABI break, I would not want that shipping for .NET 10 (maybe do this change in January for .NET 11?), and it isn't necessary.
There was a problem hiding this comment.
Thanks for the suggestions, I closed this PR now in favor of a simpler change that doesn't break the ABI.
|
Closing in favor of #10503 |
…#10503) The problem is that the Marshal.GetFunctionPointerForDelegate method does not work with delegates with generic types, such as `Action<...>` or `Func<...>` (context: dotnet/runtime#32963), which was the case of all the delegates created in `Mono.Android.Export` through `DynamicMethod`. This PR replaces the use of these generic delegate types with dynamically built non-generic types. It is based on an existing implementation in java-interop (see [here][0]). The CoreCLR is able to marshal the delegates built with these types to function pointers just fine. In a previous attempt at fixing this issue (#10493), I chose a solution which modified the shape of `JniNativeMethodRegistration` which would be an ABI breaking change (dotnet/java-interop#1364). This implementation is much simpler and doesn't cause the same problem. Thanks to @jonpryor for steering me in this direction. [0]: https://github.com/dotnet/java-interop/blob/02bceb03f7c07858590d930ef507745a88200a48/src/Java.Interop.Export/Java.Interop/MarshalMemberBuilder.cs#L274-L311
Fixes #10472
Depends on dotnet/java-interop#1364
Currently, we're running into an issue where the delegates for
[Export]methods are created usingDynamicMethodwithAction<...>orFunc<...>generic delegate types. These delegates cannot be marshalled to native function pointers becauseMarshal.GetFunctionPointerForDelegate(...), which is called internally in the runtime when aJniNativeMethodRegistrationstruct instance is passed as an argument to a p/invoke, does not support delegates with generic types in CoreCLR (context: dotnet/runtime#32963).To work around this issue, we can dynamically create a new type with a new static method using
TypeBuilderandMethodBuilder. Since this method is not a generic delegate, we can obtain a native function pointer to this method.This change needs changing the
JniNativeMethodRegistrationstruct in Java.Interop (dotnet/java-interop#1364) because it now needs to hold anIntPtrinstead of aDelegate.