[ffigen] Protocol polish

pkgs/ffigen/test/native_objc_test/protocol_config.yaml

@@ -5,6 +5,12 @@ output:
   bindings: 'protocol_bindings.dart'
   objc-bindings: 'protocol_bindings.m'
 exclude-all-by-default: true
+functions:
+  include:
+    - getClass
+    - getClassName
+    - objc_autoreleasePoolPop
+    - objc_autoreleasePoolPush
 objc-interfaces:
   include:
     - ProtocolConsumer

pkgs/ffigen/test/native_objc_test/protocol_test.dart

@@ -24,13 +24,15 @@ typedef VoidMethodBlock = ObjCBlock_ffiVoid_ffiVoid_Int32;
 typedef OtherMethodBlock = ObjCBlock_Int32_ffiVoid_Int32_Int32_Int32_Int32;
 
 void main() {
+  late ProtocolTestObjCLibrary lib;
+
   group('protocol', () {
     setUpAll(() {
       // TODO(https://github.com/dart-lang/native/issues/1068): Remove this.
       DynamicLibrary.open('../objective_c/test/objective_c.dylib');
       final dylib = File('test/native_objc_test/objc_test.dylib');
       verifySetupFile(dylib);
-      DynamicLibrary.open(dylib.absolute.path);
+      lib = ProtocolTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path));
       generateBindingsForCoverage('protocol');
     });
 
@@ -413,6 +415,71 @@ void main() {
       expect(UnusedProtocol.conformsTo(inst), isFalse);
     });
 
+    test('Threading stress test', () async {
+      final consumer = ProtocolConsumer();
+      final completer = Completer<void>();
+      int count = 0;
+
+      final protocolBuilder = ObjCProtocolBuilder();
+      MyProtocol.voidMethod_.implementAsListener(protocolBuilder, (int x) {
+        expect(x, 123);
+        ++count;
+        if (count == 1000) completer.complete();
+      });
+
+      final protocol = protocolBuilder.build();
+      final MyProtocol asMyProtocol = MyProtocol.castFrom(protocol);
+
+      for (int i = 0; i < 1000; ++i) {
+        consumer.callMethodOnRandomThread_(asMyProtocol);
+      }
+      await completer.future;
+      expect(count, 1000);
+    });
+
+    (NSObject, Pointer<ObjCBlockImpl>) blockRefCountTestInner() {
+      final pool = lib.objc_autoreleasePoolPush();
+      final protocolBuilder = ObjCProtocolBuilder();
+
+      final block = InstanceMethodBlock.fromFunction(
+          (Pointer<Void> p, NSString s, double x) => 'Hello'.toNSString());
+      MyProtocol.instanceMethod_withDouble_
+          .implementWithBlock(protocolBuilder, block);
+      final protocol = protocolBuilder.build();
+      lib.objc_autoreleasePoolPop(pool);
+
+      final blockPtr = block.ref.pointer;
+
+      // There are 2 references to the block. One owned by the Dart wrapper
+      // object, and the other owned by the protocol.
+      doGC();
+      expect(blockRetainCount(blockPtr), 2);
+
+      return (protocol, blockPtr);
+    }
+
+    Pointer<ObjCBlockImpl> blockRefCountTest() {
+      final (protocol, blockPtr) = blockRefCountTestInner();
+
+      // The Dart side block pointer has gone out of scope, but the protocol
+      // still owns a reference to it.
+      doGC();
+      expect(blockRetainCount(blockPtr), 1);
+
+      expect(protocol, isNotNull); // Force protocol to stay in scope.
+
+      return blockPtr;
+    }
+
+    test('Block ref counting', () {
+      final blockPtr = blockRefCountTest();
+
+      // The protocol object has gone out of scope, so it should be cleaned up.
+      // So should the block.
+      doGC();
+      expect(blockRetainCount(blockPtr), 0);
+    }, skip: !canDoGC);
+
     test('keepIsolateAlive', () async {
       final isolateSendPort = Completer<SendPort>();
       final protosCreated = Completer<void>();
@@ -475,5 +542,70 @@ void main() {
 
       receivePort.close();
     }, skip: !canDoGC);
+
+    test('class disposal, builder first', () {
+      final pool = lib.objc_autoreleasePoolPush();
+      ObjCProtocolBuilder? protocolBuilder =
+          ObjCProtocolBuilder(debugName: 'Foo');
+
+      NSObject? protocol = protocolBuilder.build();
+      final clazz = lib.getClass(protocol);
+      expect(lib.getClassName(clazz).cast<Utf8>().toDartString(),
+          startsWith('Foo'));
+      expect(isValidClass(clazz), isTrue);
+      lib.objc_autoreleasePoolPop(pool);
+
+      protocolBuilder = null;
+      doGC();
+      expect(isValidClass(clazz), isTrue);
+
+      protocol = null;
+      doGC();
+      expect(isValidClass(clazz), isFalse);
+    }, skip: !canDoGC);
+
+    test('class disposal, instance first', () {
+      final pool = lib.objc_autoreleasePoolPush();
+      ObjCProtocolBuilder? protocolBuilder =
+          ObjCProtocolBuilder(debugName: 'Foo');
+
+      NSObject? protocol = protocolBuilder.build();
+      final clazz = lib.getClass(protocol);
+      expect(lib.getClassName(clazz).cast<Utf8>().toDartString(),
+          startsWith('Foo'));
+      expect(isValidClass(clazz), isTrue);
+      lib.objc_autoreleasePoolPop(pool);
+
+      protocolBuilder = null;
+      doGC();
+      expect(isValidClass(clazz), isTrue);
+
+      protocol = null;
+      doGC();
+      expect(isValidClass(clazz), isFalse);
+    }, skip: !canDoGC);
+
+    test('adding more methods after build', () {
+      final protocolBuilder = ObjCProtocolBuilder();
+
+      MyProtocol.addToBuilder(
+        protocolBuilder,
+        instanceMethod_withDouble_: (NSString s, double x) {
+          return 'ProtocolBuilder: ${s.toDartString()}: $x'.toNSString();
+        },
+        optionalMethod_: (SomeStruct s) {
+          return s.y - s.x;
+        },
+      );
+
+      final protocolImpl = protocolBuilder.build();
+
+      expect(
+          () => SecondaryProtocol.addToBuilder(protocolBuilder,
+                  otherMethod_b_c_d_: (int a, int b, int c, int d) {
+                return a * b * c * d;
+              }),
+          throwsA(isA<StateError>()));
+    });
   });
 }

pkgs/ffigen/test/native_objc_test/protocol_test.h

@@ -5,6 +5,11 @@
 #import <Foundation/NSObject.h>
 #import <Foundation/NSString.h>
 
+const char* getClassName(void* cls);
+void* getClass(id object);
+void objc_autoreleasePoolPop(void *pool);
+void *objc_autoreleasePoolPush();
+
 typedef struct {
   int32_t x;
   int32_t y;

pkgs/ffigen/test/native_objc_test/protocol_test.m

@@ -8,6 +8,16 @@
 
 #include "protocol_test.h"
 
+const char* class_getName(Class cls);
+
+const char* getClassName(void* cls) {
+  return class_getName((__bridge Class)cls);
+}
+
+void* getClass(id object) {
+  return (__bridge void*)[object class];
+}
+
 @implementation ProtocolConsumer : NSObject
 - (NSString*)callInstanceMethod:(id<SuperProtocol>)protocol {
   return [protocol instanceMethod:@"Hello from ObjC" withDouble:3.14];

pkgs/ffigen/test/native_objc_test/util.dart

@@ -93,3 +93,6 @@ int objectRetainCount(Pointer<ObjCObject> object) {
   if (!internal_for_testing.isValidClass(clazz)) return 0;
   return _getObjectRetainCount(object.cast());
 }
+
+bool isValidClass(Pointer<Void> clazz) =>
+    internal_for_testing.isValidClass(clazz.cast(), bypassCache: true);

pkgs/objective_c/ffigen_c.yaml

@@ -22,7 +22,6 @@ functions:
     - 'protocol_getMethodDescription'
     - 'protocol_getName'
     - 'newFinalizableHandle'
-    - 'class_addMethod'
     - 'DOBJC_.*'
   leaf:
     include:
@@ -56,10 +55,7 @@ functions:
     'objc_msgSend_stret': 'msgSendStret'
     'object_getClass': 'getObjectClass'
     'objc_copyClassList': 'copyClassList'
-    'objc_allocateClassPair': 'allocateClassPair'
-    'objc_registerClassPair': 'registerClassPair'
     'objc_getProtocol': 'getProtocol'
-    'class_addMethod': 'addMethod'
     'protocol_getMethodDescription': 'getMethodDescription'
     'protocol_getName': 'getProtocolName'
 globals:

pkgs/objective_c/lib/src/c_bindings_generated.dart

@@ -56,30 +56,6 @@ external ffi.Array<ffi.Pointer<ffi.Void>> NSConcreteMallocBlock;
 @ffi.Native<ffi.Array<ffi.Pointer<ffi.Void>>>(symbol: '_NSConcreteStackBlock')
 external ffi.Array<ffi.Pointer<ffi.Void>> NSConcreteStackBlock;
 
-@ffi.Native<
-    ffi.Bool Function(
-        ffi.Pointer<ObjCObject>,
-        ffi.Pointer<ObjCSelector>,
-        ffi.Pointer<ffi.Void>,
-        ffi.Pointer<ffi.Char>)>(symbol: 'class_addMethod', isLeaf: true)
-external bool addMethod(
-  ffi.Pointer<ObjCObject> cls,
-  ffi.Pointer<ObjCSelector> name,
-  ffi.Pointer<ffi.Void> imp,
-  ffi.Pointer<ffi.Char> types,
-);
-
-@ffi.Native<
-    ffi.Pointer<ObjCObject> Function(
-        ffi.Pointer<ObjCObject>,
-        ffi.Pointer<ffi.Char>,
-        ffi.Size)>(symbol: 'objc_allocateClassPair', isLeaf: true)
-external ffi.Pointer<ObjCObject> allocateClassPair(
-  ffi.Pointer<ObjCObject> superclass,
-  ffi.Pointer<ffi.Char> name,
-  int extraBytes,
-);
-
 @ffi.Native<ffi.Void Function(ffi.Pointer<ffi.Void>)>(
     symbol: 'DOBJC_awaitWaiter')
 external void awaitWaiter(
@@ -211,12 +187,6 @@ external ffi.Pointer<ObjCObject> objectRetain(
   ffi.Pointer<ObjCObject> object,
 );
 
-@ffi.Native<ffi.Void Function(ffi.Pointer<ObjCObject>)>(
-    symbol: 'objc_registerClassPair', isLeaf: true)
-external void registerClassPair(
-  ffi.Pointer<ObjCObject> cls,
-);
-
 @ffi.Native<ffi.Pointer<ObjCSelector> Function(ffi.Pointer<ffi.Char>)>(
     symbol: 'sel_registerName', isLeaf: true)
 external ffi.Pointer<ObjCSelector> registerName(

pkgs/objective_c/lib/src/internal.dart

@@ -295,8 +295,8 @@ bool _isValidObject(ObjectPtr ptr) {
 
 final _allClasses = <ObjectPtr>{};
 
-bool _isValidClass(ObjectPtr clazz) {
-  if (_allClasses.contains(clazz)) return true;
+bool _isValidClass(ObjectPtr clazz, {bool bypassCache = false}) {
+  if (!bypassCache && _allClasses.contains(clazz)) return true;
 
   // If the class is missing from the list, it either means we haven't created
   // the set yet, or more classes have been loaded since we created the set, or
@@ -446,5 +446,6 @@ BlockPtr wrapBlockingBlock(
 bool blockHasRegisteredClosure(BlockPtr block) =>
     _blockClosureRegistry.containsKey(block.ref.target.address);
 bool isValidBlock(BlockPtr block) => c.isValidBlock(block);
-bool isValidClass(ObjectPtr clazz) => _isValidClass(clazz);
+bool isValidClass(ObjectPtr clazz, {bool bypassCache = false}) =>
+    _isValidClass(clazz, bypassCache: bypassCache);
 bool isValidObject(ObjectPtr object) => _isValidObject(object);