Obj-C native trampoline file should support ARC

[stuartmorgan]

In my experimentation with adopting FFI in video_player, I ended up with a generated trampoline file. The CHANGELOG for ffigen says:

If you have listener blocks affected by this ref count bug, a .m file will be generated containing the trampoline. You must compile this .m file into your package. If you already have a flutter plugin or build.dart, you can simply add this generated file to that build.

I would be shocked if more than a handful of existing Flutter plugins and apps (if any) are built without ARC, and it's absolutely not something we would want to suggest to people. So that means that "simply" adding that file to an existing build, in the context of Flutter, is not simple at all—it requires learning about the difference between ARC and non-ARC code (at this point, I would expect that a large majority of iOS developers do not have non-ARC experience), and then if it's a plugin how to opt specific files out of ARC in both CocoaPods and Swift Package Manager.

I see there was previous discussion of this in https://github.com/dart-lang/native/issues/1360, but I don't think that telling each developer to handle this is the right outcome if the goal is to make interop seamless. ARC is the default assumption for any modern compilation, so we should generate code that works under ARC.

[stuartmorgan]

and Swift Package Manager

Actually, it appears that SPM does not support disabling ARC. I don't see any options for it, and https://forums.swift.org/t/support-for-disabling-arc/27998 is still open.

Since Flutter is moving its plugin system to ARC, it looks like this is a hard blocker for adopting FFI in Flutter plugins (if they have APIs requiring a trampoline).

[dcharkes]

I see there was previous discussion of this in #1360, but I don't think that telling each developer to handle this is the right outcome if the goal is to make interop seamless.

It would be (almost) seamless when we have the native assets feature. At that point we need to tell the user to have a hook/build.dart and we can provide a helper function that can be called in that hook that compiles (and bundles) the code that does the refcounting.

@mkustermann @liamappelbe @HosseinYousefi and I have discussed various approaches for ref counting. Some notes of our discussions can be found in https://github.com/dart-lang/native/issues/835. Implementing shared memory multi-threading would enable us to increase refcounts synchronously from Dart https://github.com/dart-lang/sdk/issues/54530.

[stuartmorgan]

It would be (almost) seamless when we have the native assets feature. At that point we need to tell the user to have a hook/build.dart and we can provide a helper function that can be called in that hook that compiles (and bundles) the code that does the refcounting.

How would having to maintain two different sets of native source code with two different sets of build files be "almost seamless"?

[dcharkes]

WDYM maintain? It's code generated by FFIgen.

The larger design approach is that for every single thing that cannot be expressed in Dart (writing adaptors for native finalizers with different signatures, capturing errno, doing refcounting for ObjC, global ref releases for JVM) we should be a feature into dart:ffi, or alternatively we make it as simple as possible to generate wrapper code and have it automatically built and bundled. (I don't believe this is much different from Pigeon where the base is message passing instead of FFI. The other things are built on top of the base layer, not into the base layer.)

(Technical lore: We explored compiling a subset of Dart not using isolates or the heap to C. https://github.com/dart-lang/sdk/issues/47778. But then decided that writing that code in the native language and having a build and bundling system for that, "native assets", would be a more general and easier solution.)

[stuartmorgan]

WDYM maintain? It's code generated by FFIgen.

I mean that as a plugin developer I would have two different sets of native code, one generated by ffigen, and the other authored by me, and I would have two sets of build systems to manage them, and I would have to set up and maintain both of those build systems and keep track of which files go in which one. Even if it's just one source file, adding a second build system adds ongoing complexity.

The larger design approach is that for every single thing that cannot be expressed in Dart (writing adaptors for native finalizers with different signatures, capturing errno, doing refcounting for ObjC, global ref releases for JVM) we should be a feature into dart:ffi, or alternatively we make it as simple as possible to generate wrapper code and have it automatically built and bundled.

I have yet to see a design proposal that would address https://github.com/flutter/flutter/issues/110353. Addressing that is a goal to work toward, but it won't be solved by native assets.

When I say "a hard blocker for adopting FFI in Flutter plugins", I'm referring specifically to the thing that has traditionally been called a Flutter plugin, with a native build system managed by the flutter tool. Cases that don't need any plugin APIs and can be fully converted to FFI packages that don't use the Flutter plugin system once native assets are available wouldn't be affected.

Currently, however, almost all of the Flutter-team-maintained plugins are not that case, due to requiring integration with some part of the Flutter plugin API surface.

(I don't believe this is much different from Pigeon where the base is message passing instead of FFI. The other things are built on top of the base layer, not into the base layer.)

I'm not sure what the relevance of Pigeon is here; Pigeon doesn't generate non-ARC code. If it did, we would consider that a serious issue, for exactly the reason described in my initial report: it would make including it in the build system we are telling people to include it in very difficult.

[liamappelbe]

@stuartmorgan These trampolines exist to fix #835. TLDR: when ObjC calls back into Dart using an async callback, any refcounted args it passes may be deleted before the Dart code is invoked. To solve this, these trampolines increment the refcount synchronously in ObjC land before invoking the async Dart callback.

I don't know how to port this behavior to ARC. Can we manually increment the ref counts in ARC? Presumably this must be possible, to handle analogous cases that can occur in pure ObjC (eg the callback needs to pass a refcounted arg to another thread). Maybe if I directly call the underlying objc_retain function, that would bypass this check?

If there's no way of porting these trampolines, then the other option is to prioritize NativeCallable.temporaryIsolate, which would let us write these trampolines in Dart.

[stuartmorgan]

Can you just use bridge casts to indicate the transfer of ownership to/from ARC?

[liamappelbe]

Can you just use bridge casts to indicate the transfer of ownership to/from ARC?

Yeah, that should work, with some hackery. They're designed to convert retainable types to non-retainable types, not to just increment a ref count without changing the type, but it's doable: (__bridge FooInterface*)(__bridge_retained void*)foo

Using a __bridge_retained or __bridge_transfer cast purely to convince ARC to emit an unbalanced retain or release, respectively, is poor form.

😆

[liamappelbe]

Just calling objc_retain seems to work, and is more readable than the bridge casts

[liamappelbe]

The hard part is migrating all the existing ref counting tests to ARC. They're doing a lot of manual ref count management and I'm having a lot of trouble reasoning about the ref counts under ARC. Might be better to compile the test ObjC code without ARC, while still compiling the trampoline file with ARC.

[stuartmorgan]

Might be better to compile the test ObjC code without ARC, while still compiling the trampoline file with ARC.

Seems reasonable, although longer term the tests will probably less fragile if they can be written in a way that works under ARC (i.e., by asserting things about lifetime using weak pointers, rather than asserting specific ref counts, since those are subject to change by ARC).