[ffigen] Release all ObjC references on the main thread (#1603)

.github/workflows/ffigen.yml

@@ -37,7 +37,7 @@ jobs:
           channel: 'stable'
       - id: install
         name: Install dependencies
-        run: flutter pub get && flutter pub get --directory="example/shared_bindings"
+        run: flutter pub get && flutter pub get --directory="example/shared_bindings" && flutter pub get --directory="../objective_c"
       - name: Check formatting
         run: dart format --output=none --set-exit-if-changed .
         if: always() && steps.install.outcome == 'success'
@@ -79,7 +79,7 @@ jobs:
         with:
           channel: 'stable'
       - name: Install dependencies
-        run: flutter pub get
+        run: flutter pub get && flutter pub get --directory="../objective_c"
       - name: Build test dylib and bindings
         run: dart test/setup.dart
       - name: Run VM tests and collect coverage
@@ -110,9 +110,9 @@ jobs:
         with:
           channel: 'stable'
       - name: Install dependencies
-        run: flutter pub get
+        run: flutter pub get && flutter pub get --directory="../objective_c"
       - name: Build test dylib and bindings
-        run: dart test/setup.dart
+        run: dart test/setup.dart --main-thread-dispatcher
       - name: Run Flutter tests
         run: flutter test
 

.github/workflows/ffigen_weekly.yml

@@ -25,7 +25,7 @@ jobs:
           flutter-version: 3.19.0
           channel: 'stable'
       - name: Install dependencies
-        run: flutter pub get
+        run: flutter pub get && flutter pub get --directory="../objective_c"
       - name: Build test dylib and bindings
         run: dart test/setup.dart
       - name: Run VM tests

pkgs/ffigen/test/native_objc_test/arc_config.yaml

@@ -1,7 +1,9 @@
 name: ArcTestObjCLibrary
 description: 'Tests ARC'
 language: objc
-output: 'arc_bindings.dart'
+output:
+  bindings: 'arc_bindings.dart'
+  objc-bindings: 'arc_bindings.m'
 exclude-all-by-default: true
 functions:
   include:
@@ -10,8 +12,9 @@ functions:
 objc-interfaces:
   include:
     - ArcTestObject
+    - ArcDtorTestObject
 headers:
   entry-points:
-    - 'arc_test.m'
+    - 'arc_test.h'
 preamble: |
   // ignore_for_file: camel_case_types, non_constant_identifier_names, unnecessary_non_null_assertion, unused_element, unused_field

pkgs/ffigen/test/native_objc_test/arc_test.dart

@@ -20,7 +20,7 @@ import 'util.dart';
 void main() {
   late ArcTestObjCLibrary lib;
 
-  group('Reference counting', () {
+  group('ARC', () {
     setUpAll(() {
       // TODO(https://github.com/dart-lang/native/issues/1068): Remove this.
       DynamicLibrary.open('../objective_c/test/objective_c.dylib');
@@ -474,5 +474,23 @@ void main() {
       expect(counter.value, 0);
       calloc.free(counter);
     }, skip: !canDoGC);
+
+    test('Destroy on main thread', () async {
+      const numTestObjects = 1000;
+
+      final dtorCounter = calloc<Int32>();
+      final dtorOnMainThreadCounter = calloc<Int32>();
+      final objects = <ArcDtorTestObject>[];
+      for (var i = 0; i < numTestObjects; ++i) {
+        objects.add(ArcDtorTestObject.alloc().initWithCounters_onMainThread_(
+            dtorCounter, dtorOnMainThreadCounter));
+      }
+      objects.clear();
+
+      while (dtorCounter.value < numTestObjects) {
+        await flutterDoGC();
+      }
+      expect(dtorOnMainThreadCounter.value, numTestObjects);
+    }, skip: !isFlutterTester);
   });
 }

pkgs/ffigen/test/native_objc_test/arc_test.h

@@ -0,0 +1,41 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#import <Foundation/NSObject.h>
+
+void objc_autoreleasePoolPop(void *pool);
+void *objc_autoreleasePoolPush();
+
+@interface ArcTestObject : NSObject {
+  int32_t* counter;
+}
+
++ (instancetype)allocTheThing;
++ (instancetype)newWithCounter:(int32_t*) _counter;
+- (instancetype)initWithCounter:(int32_t*) _counter;
++ (ArcTestObject*)makeAndAutorelease:(int32_t*) _counter;
+- (void)setCounter:(int32_t*) _counter;
+- (void)dealloc;
+- (ArcTestObject*)copyMe;
+- (ArcTestObject*)mutableCopyMe;
+- (id)copyWithZone:(NSZone*) zone;
+- (ArcTestObject*)returnsRetained NS_RETURNS_RETAINED;
+- (ArcTestObject*)copyMeNoRetain __attribute__((ns_returns_not_retained));
+- (ArcTestObject*)copyMeAutorelease __attribute__((ns_returns_autoreleased));
+- (ArcTestObject*)copyMeConsumeSelf __attribute__((ns_consumes_self));
++ (void)consumeArg:(ArcTestObject*) __attribute((ns_consumed)) arg;
+
+@property (assign) ArcTestObject* assignedProperty;
+@property (retain) ArcTestObject* retainedProperty;
+@property (copy) ArcTestObject* copiedProperty;
+
+@end
+
+@interface ArcDtorTestObject : NSObject {
+  int32_t* dtorCounter;
+  int32_t* dtorOnMainThreadCounter;
+}
+- (instancetype)initWithCounters:(int32_t*) _dtorCounter
+    onMainThread: (int32_t*) _dtorOnMainThreadCounter;
+@end

pkgs/ffigen/test/native_objc_test/arc_test.m

@@ -2,42 +2,15 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-#import <Foundation/NSObject.h>
+#import <Foundation/NSThread.h>
 
+#include "arc_test.h"
 #include "util.h"
 
 #if !__has_feature(objc_arc)
 #error "This file must be compiled with ARC enabled"
 #endif
 
-void objc_autoreleasePoolPop(void *pool);
-void *objc_autoreleasePoolPush();
-
-@interface ArcTestObject : NSObject {
-  int32_t* counter;
-}
-
-+ (instancetype)allocTheThing;
-+ (instancetype)newWithCounter:(int32_t*) _counter;
-- (instancetype)initWithCounter:(int32_t*) _counter;
-+ (ArcTestObject*)makeAndAutorelease:(int32_t*) _counter;
-- (void)setCounter:(int32_t*) _counter;
-- (void)dealloc;
-- (ArcTestObject*)copyMe;
-- (ArcTestObject*)mutableCopyMe;
-- (id)copyWithZone:(NSZone*) zone;
-- (ArcTestObject*)returnsRetained NS_RETURNS_RETAINED;
-- (ArcTestObject*)copyMeNoRetain __attribute__((ns_returns_not_retained));
-- (ArcTestObject*)copyMeAutorelease __attribute__((ns_returns_autoreleased));
-- (ArcTestObject*)copyMeConsumeSelf __attribute__((ns_consumes_self));
-+ (void)consumeArg:(ArcTestObject*) __attribute((ns_consumed)) arg;
-
-@property (assign) ArcTestObject* assignedProperty;
-@property (retain) ArcTestObject* retainedProperty;
-@property (copy) ArcTestObject* copiedProperty;
-
-@end
-
 @implementation ArcTestObject
 
 + (instancetype)allocTheThing {
@@ -98,3 +71,20 @@ - (ArcTestObject*)copyMeConsumeSelf __attribute__((ns_consumes_self)) {
 + (void)consumeArg:(ArcTestObject*) __attribute((ns_consumed)) arg {}
 
 @end
+
+@implementation ArcDtorTestObject
+
+- (instancetype)initWithCounters:(int32_t*) _dtorCounter
+    onMainThread: (int32_t*) _dtorOnMainThreadCounter {
+  dtorCounter = _dtorCounter;
+  dtorOnMainThreadCounter = _dtorOnMainThreadCounter;
+  return [super init];
+}
+
+- (void)dealloc {
+  ++*dtorCounter;
+  if ([NSThread isMainThread]) {
+    ++*dtorOnMainThreadCounter;
+  }
+}
+@end

pkgs/ffigen/test/native_objc_test/setup.dart

@@ -5,6 +5,8 @@
 import 'dart:async';
 import 'dart:io';
 
+import 'package:args/args.dart';
+
 // All ObjC source files are compiled with ARC enabled except these.
 const arcDisabledFiles = <String>{
   'ref_count_test.m',
@@ -152,16 +154,24 @@ Future<void> clean(List<String> testNames) async {
 }
 
 Future<void> main(List<String> arguments) async {
+  final parser = ArgParser();
+  parser.addFlag('clean');
+  parser.addFlag('main-thread-dispatcher');
+  final args = parser.parse(arguments);
+
   // Allow running this script directly from any path (or an IDE).
   Directory.current = Platform.script.resolve('.').toFilePath();
   if (!Platform.isMacOS) {
     throw OSError('Objective C tests are only supported on MacOS');
   }
 
-  if (arguments.isNotEmpty && arguments[0] == 'clean') {
+  if (args.flag('clean')) {
     return await clean(_getTestNames());
   }
 
-  await _runDart(['../objective_c/test/setup.dart']);
-  return await build(arguments.isNotEmpty ? arguments : _getTestNames());
+  await _runDart([
+    '../objective_c/test/setup.dart',
+    if (args.flag('main-thread-dispatcher')) '--main-thread-dispatcher',
+  ]);
+  return await build(args.rest.isNotEmpty ? args.rest : _getTestNames());
 }

pkgs/ffigen/test/setup.dart

@@ -8,10 +8,12 @@
 import 'dart:async';
 import 'dart:io';
 
-Future<void> _run(String subdir, String script) async {
+import 'package:args/args.dart';
+
+Future<void> _run(String subdir, String script, List<String> flags) async {
   final dir = Platform.script.resolve('$subdir/');
   print('\nRunning $script in ${dir.toFilePath()}');
-  final args = ['run', dir.resolve(script).toFilePath()];
+  final args = ['run', dir.resolve(script).toFilePath(), ...flags];
   final process = await Process.start(
     Platform.executable,
     args,
@@ -25,10 +27,16 @@ Future<void> _run(String subdir, String script) async {
   }
 }
 
-Future<void> main() async {
-  await _run('native_test', 'build_test_dylib.dart');
+Future<void> main(List<String> arguments) async {
+  final parser = ArgParser();
+  parser.addFlag('main-thread-dispatcher');
+  final args = parser.parse(arguments);
+
+  await _run('native_test', 'build_test_dylib.dart', []);
   if (Platform.isMacOS) {
-    await _run('native_objc_test', 'setup.dart');
+    await _run('native_objc_test', 'setup.dart', [
+      if (args.flag('main-thread-dispatcher')) '--main-thread-dispatcher',
+    ]);
   }
   print('\nSuccess :)\n');
 }

pkgs/objective_c/CHANGELOG.md

@@ -3,6 +3,7 @@
 - Add `UnimplementedOptionalMethodException`, which is thrown by the ObjC
   bindings if an optional method is invoked, and the instance doesn't implement
   the method.
+- Dispatch all object/block releases to the main thread.
 
 ## 2.0.0
 

pkgs/objective_c/ios/Classes/objective_c.m

@@ -4,5 +4,6 @@
 
 // Relative import to be able to reuse the ObjC sources.
 // See the comment in ../objective_c.podspec for more information.
+#include "../../src/objective_c.m"
 #include "../../src/objective_c_bindings_generated.m"
 #include "../../src/proxy.m"

pkgs/objective_c/lib/src/objective_c_bindings_generated.dart.m

@@ -0,0 +1,26 @@
+#include <stdint.h>
+#import "../../src/foundation.h"
+#import "../../src/proxy.h"
+
+#if !__has_feature(objc_arc)
+#error "This file must be compiled with ARC enabled"
+#endif
+
+id objc_retain(id);
+id objc_retainBlock(id);
+
+typedef void  (^_ListenerTrampoline)(void * arg0, id arg1);
+_ListenerTrampoline _wrapListenerBlock_3f15836a(_ListenerTrampoline block) NS_RETURNS_RETAINED {
+  return ^void(void * arg0, id arg1) {
+    objc_retainBlock(block);
+    block(arg0, objc_retain(arg1));
+  };
+}
+
+typedef void  (^_ListenerTrampoline1)(void * arg0);
+_ListenerTrampoline1 _wrapListenerBlock_162449db(_ListenerTrampoline1 block) NS_RETURNS_RETAINED {
+  return ^void(void * arg0) {
+    objc_retainBlock(block);
+    block(arg0);
+  };
+}

pkgs/objective_c/macos/Classes/objective_c.m

@@ -4,5 +4,6 @@
 
 // Relative import to be able to reuse the ObjC sources.
 // See the comment in ../objective_c.podspec for more information.
+#include "../../src/objective_c.m"
 #include "../../src/objective_c_bindings_generated.m"
 #include "../../src/proxy.m"

pkgs/objective_c/pubspec.yaml

@@ -25,6 +25,7 @@ dependencies:
   yaml: ^3.1.0
 
 dev_dependencies:
+  args: ^2.0.0
   coverage: ^1.8.0
   dart_flutter_team_lints: ^2.0.0
   ffigen: ^14.0.0

pkgs/objective_c/src/CMakeLists.txt

@@ -7,6 +7,8 @@ project(objective_c_library VERSION 0.0.1 LANGUAGES C)
 
 add_library(objective_c SHARED
   "objective_c.c"
+  "objective_c.m"
+  "proxy.m"
   "include/dart_api_dl.c"
 )
 

pkgs/objective_c/src/objective_c.c

@@ -25,21 +25,19 @@ bool isValidBlock(ObjCBlockImpl* block) {
 
 void finalizeObject(void* isolate_callback_data, void* peer) {
   // objc_release works for Objects and Blocks.
-  objc_release(peer);
+  runOnMainThread((void (*)(void*))objc_release, peer);
 }
 
 Dart_FinalizableHandle newFinalizableHandle(Dart_Handle owner,
-                                                  ObjCObject* object) {
+                                            ObjCObject* object) {
   return Dart_NewFinalizableHandle_DL(owner, object, 0, finalizeObject);
 }
 
 void deleteFinalizableHandle(Dart_FinalizableHandle handle, Dart_Handle owner) {
   Dart_DeleteFinalizableHandle_DL(handle, owner);
 }
 
-void finalizeMalloc(void* isolate_callback_data, void* peer) {
-  free(peer);
-}
+void finalizeMalloc(void* isolate_callback_data, void* peer) { free(peer); }
 
 bool* newFinalizableBool(Dart_Handle owner) {
   bool* pointer = (bool*)malloc(1);

pkgs/objective_c/src/objective_c.h

@@ -28,4 +28,13 @@ void deleteFinalizableHandle(Dart_FinalizableHandle handle, Dart_Handle owner);
 // by a Dart_FinalizableHandle when the owner is garbage collected.
 bool* newFinalizableBool(Dart_Handle owner);
 
+// Runs fn(arg) on the main thread. If runOnMainThread is already running on the
+// main thread, fn(arg) is invoked synchronously. Otherwise it is dispatched to
+// the main thread (ie dispatch_async(dispatch_get_main_queue(), ...)).
+//
+// This assumes that the main thread is executing its queue. If not, #define
+// NO_MAIN_THREAD_DISPATCH to disable this, and run fn(arg) synchronously. The
+// flutter runner does execute the main dispatch queue, but the Dart VM doesn't.
+void runOnMainThread(void (*fn)(void*), void* arg);
+
 #endif  // OBJECTIVE_C_SRC_OBJECTIVE_C_H_

pkgs/objective_c/src/objective_c.m

@@ -0,0 +1,20 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#import <Foundation/NSThread.h>
+#import <dispatch/dispatch.h>
+
+void runOnMainThread(void (*fn)(void*), void* arg) {
+#ifdef NO_MAIN_THREAD_DISPATCH
+  fn(arg);
+#else
+  if ([NSThread isMainThread]) {
+    fn(arg);
+  } else {
+    dispatch_async(dispatch_get_main_queue(), ^{
+      fn(arg);
+    });
+  }
+#endif
+}