NordSecurity / uniffi-bindgen-cs

C# bindings generator for uniffi-rs
https://github.com/NordSecurity/uniffi-bindgen-cs
Mozilla Public License 2.0
109 stars 21 forks source link

Interop fails on iOS devices #83

Closed AArnott closed 3 months ago

AArnott commented 3 months ago

The uniffi layer works great, except on iOS, which doesn't allow JIT. Everything must be AOT compiled.

At runtime, the app crashes with this exception:  

Attempting to JIT compile method '(wrapper native-to-managed) int uniffi.LightWallet.ForeignCallbackTypeCancellationSource/<>c:<.cctor>b__3_0 (ulong,uint,intptr,int,uniffi.LightWallet.RustBuffer&)' while running in aot-only mode. See https://docs.microsoft.com/xamarin/ios/internals/limitations for more information.

The relevant method in the uniffi generated code is here:

https://github.com/nerdcash/Nerdbank.Cryptocurrencies/blob/9da7aa7d85e0b17080535baaaa9265c7c4aaa326/src/Nerdbank.Zcash/RustBindings/LightWallet.cs#L2278-L2333

I'm currently looking for a way to resolve this, and am curious whether anyone else has come across this or whether a solution might exist by generating modified code.

AArnott commented 3 months ago

Relevant docs: https://learn.microsoft.com/en-us/previous-versions/xamarin/ios/internals/limitations#using-delegates-to-call-native-functions

AArnott commented 3 months ago

I have a fix!

Make the following changes to the callbacks:

 class ForeignCallbackTypeSyncUpdate
 {
-       // This cannot be a static method. Although C# supports implicitly using a static method as a
-       // delegate, the behaviour is incorrect for this use case. Using static method as a delegate
-       // argument creates an implicit delegate object, that is later going to be collected by GC. Any
-       // attempt to invoke a garbage collected delegate results in an error:
-       //   > A callback was made on a garbage collected delegate of type 'ForeignCallback::..'
-       public static ForeignCallback INSTANCE = (
+       public static readonly ForeignCallback INSTANCE = INSTANCE_FUNC;
+
+#if IOS
+       [ObjCRuntime.MonoPInvokeCallback(typeof(ForeignCallback))]
+#endif
+       private static int INSTANCE_FUNC(
                ulong handle,
                uint method,
                IntPtr argsData,
                int argsLength,
                ref RustBuffer outBuf
-       ) =>
+       )
        {
                var cb = FfiConverterTypeSyncUpdate.INSTANCE.Lift(handle);
                switch (method)
@@ -2451,7 +2454,7 @@ class ForeignCallbackTypeSyncUpdate
                                return UniffiCallbackResponseCode.UNEXPECTED_ERROR;
                        }
                }
-       };
+       }

The code comment that was emitted before isn't exactly true. Static methods do work. But you also need to define and retain a delegate instance. The [MonoPInvokeCallback] attribute is required so that the mono AOT compiler knows what interop stub to generate, and the attribute can only be applied to methods. So the fix involves defining the static method as a method rather than as a lambda, applying the attribute, and then defining a static readonly field to store the delegate that must be retained and used. Minimal code changes.

The only awkward bit is the #if IOS bit, which is required because the attribute is only defined for apple targets.

At the moment, this means I have to manually edit the generated C# file to make these changes. I'd love to get an update to this repo to make this built-in. How quickly can we have this? Would offering a PR help?

arg0d commented 3 months ago

Hey, I don't see any problem with merging this kind of PR. Could you submit a PR?