Partial fix for block ref counts

dart-archive · Oct 26, 2023 · 2f41968 · 2f41968
1 parent 6637967
commit 2f41968
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 34 deletions.
diff --git a/lib/src/code_generator/objc_block.dart b/lib/src/code_generator/objc_block.dart
@@ -118,19 +118,22 @@ $returnFfiDartType $closureTrampoline($blockCType block, $paramsFfiDartType) =>
     // Snippet that converts a Dart typed closure to FfiDart type. This snippet
     // is used below. Note that the closure being converted is called `fn`.
     final convertedFnArgs = params
-        .map((p) => p.type.convertFfiDartTypeToDartType(w, p.name, 'lib'))
+        .map((p) => p.type.convertFfiDartTypeToDartType(w, p.name, 'lib',
+            objCShouldRetain: false))
         .join(', ');
-    final convFnInvocation =
-        returnType.convertDartTypeToFfiDartType(w, 'fn($convertedFnArgs)');
+    final convFnInvocation = returnType.convertDartTypeToFfiDartType(
+        w, 'fn($convertedFnArgs)',
+        objCShouldRetain: true);
     final convFn = '($paramsFfiDartType) => $convFnInvocation';
 
     // Write the wrapper class.
     final defaultValue = returnType.getDefaultValue(w, '_lib');
     final exceptionalReturn = defaultValue == null ? '' : ', $defaultValue';
     s.write('''
 class $name extends _ObjCBlockBase {
-  $name._($blockCType id, ${w.className} lib) :
-      super._(id, lib, retain: false, release: true);
+  $name._($blockCType id, ${w.className} lib,
+      {bool retain = false, bool release = true}) :
+          super._(id, lib, retain: retain, release: release);
 
   /// Creates a block from a C function pointer.
   ///
@@ -189,7 +192,8 @@ class $name extends _ObjCBlockBase {
 _id.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType>()(
     _id, $callMethodArgs)''';
     s.write(returnType.convertFfiDartTypeToDartType(
-        w, callMethodInvocation, '_lib'));
+        w, callMethodInvocation, '_lib',
+        objCShouldRetain: false));
     s.write(';\n');
 
     s.write('}\n');
@@ -223,17 +227,22 @@ _id.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType>()(
   bool get sameDartAndCType => false;
 
   @override
-  String convertDartTypeToFfiDartType(Writer w, String value) => '$value._id';
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) =>
+      ObjCInterface.generateGetId(value, objCShouldRetain);
 
   @override
   String convertFfiDartTypeToDartType(
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) =>
-      '$name._($value, $library)';
+      ObjCInterface.generateConstructor(name, value, library, objCShouldRetain);
 
   @override
   String toString() => '($returnType (^)(${argTypes.join(', ')}))';

diff --git a/lib/src/code_generator/objc_built_in_functions.dart b/lib/src/code_generator/objc_built_in_functions.dart
@@ -239,6 +239,11 @@ class $name implements ${w.ffiLibraryPrefix}.Finalizable {
 
   /// Return a pointer to this object.
   $idType get pointer => _id;
+
+  $idType _retainAndReturnId() {
+    _lib.$retain(_id.cast());
+    return _id;
+  }
 }
 ''');
   }

diff --git a/lib/src/code_generator/objc_interface.dart b/lib/src/code_generator/objc_interface.dart
@@ -215,7 +215,7 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
           w,
           '_ret',
           '_lib',
-          isObjCOwnedReturn: m.isOwnedReturn,
+          objCShouldRetain: !m.isOwnedReturn,
           objCEnclosingClass: name,
         );
         s.write('    return $result;');
@@ -400,26 +400,33 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
   bool get sameDartAndCType => false;
 
   @override
-  String convertDartTypeToFfiDartType(Writer w, String value) => '$value._id';
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) =>
+      ObjCInterface.generateGetId(value, objCShouldRetain);
+
+  static String generateGetId(String value, bool objCShouldRetain) =>
+      objCShouldRetain ? '$value._retainAndReturnId()' : '$value._id';
 
   @override
   String convertFfiDartTypeToDartType(
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) =>
-      ObjCInterface.generateConstructor(
-          name, value, library, isObjCOwnedReturn);
+      ObjCInterface.generateConstructor(name, value, library, objCShouldRetain);
 
   static String generateConstructor(
     String className,
     String value,
     String library,
-    bool isObjCOwnedReturn,
+    bool objCShouldRetain,
   ) {
-    final ownershipFlags = 'retain: ${!isObjCOwnedReturn}, release: true';
+    final ownershipFlags = 'retain: $objCShouldRetain, release: true';
     return '$className._($value, $library, $ownershipFlags)';
   }
 

diff --git a/lib/src/code_generator/objc_nullable.dart b/lib/src/code_generator/objc_nullable.dart
@@ -45,11 +45,16 @@ class ObjCNullable extends Type {
   bool get sameDartAndCType => false;
 
   @override
-  String convertDartTypeToFfiDartType(Writer w, String value) {
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) {
     // This is a bit of a hack, but works for all the types that are allowed to
     // be a child type. If we add more allowed child types, we may have to start
     // special casing each type. Turns value._id into value?._id ?? nullptr.
-    final convertedValue = child.convertDartTypeToFfiDartType(w, '$value?');
+    final convertedValue = child.convertDartTypeToFfiDartType(w, '$value?',
+        objCShouldRetain: objCShouldRetain);
     return '$convertedValue ?? ${w.ffiLibraryPrefix}.nullptr';
   }
 
@@ -58,15 +63,15 @@ class ObjCNullable extends Type {
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) {
     // All currently supported child types have a Pointer as their FfiDartType.
     final convertedValue = child.convertFfiDartTypeToDartType(
       w,
       value,
       library,
-      isObjCOwnedReturn: isObjCOwnedReturn,
+      objCShouldRetain: objCShouldRetain,
       objCEnclosingClass: objCEnclosingClass,
     );
     return '$value.address == 0 ? null : $convertedValue';

diff --git a/lib/src/code_generator/pointer.dart b/lib/src/code_generator/pointer.dart
@@ -88,16 +88,21 @@ class ObjCObjectPointer extends PointerType {
   bool get sameDartAndCType => false;
 
   @override
-  String convertDartTypeToFfiDartType(Writer w, String value) => '$value._id';
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) =>
+      '$value._id';
 
   @override
   String convertFfiDartTypeToDartType(
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) =>
       ObjCInterface.generateConstructor(
-          'NSObject', value, library, isObjCOwnedReturn);
+          'NSObject', value, library, objCShouldRetain);
 }
diff --git a/lib/src/code_generator/type.dart b/lib/src/code_generator/type.dart
@@ -56,15 +56,20 @@ abstract class Type {
 
   /// Returns generated Dart code that converts the given value from its
   /// DartType to its FfiDartType.
-  String convertDartTypeToFfiDartType(Writer w, String value) => value;
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) =>
+      value;
 
   /// Returns generated Dart code that converts the given value from its
   /// FfiDartType to its DartType.
   String convertFfiDartTypeToDartType(
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) =>
       value;
@@ -124,14 +129,19 @@ abstract class BindingType extends NoLookUpBinding implements Type {
   bool get sameDartAndCType => sameFfiDartAndCType;
 
   @override
-  String convertDartTypeToFfiDartType(Writer w, String value) => value;
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) =>
+      value;
 
   @override
   String convertFfiDartTypeToDartType(
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) =>
       value;

diff --git a/lib/src/code_generator/typealias.dart b/lib/src/code_generator/typealias.dart
@@ -159,22 +159,27 @@ class Typealias extends BindingType {
   bool get sameDartAndCType => type.sameDartAndCType;
 
   @override
-  String convertDartTypeToFfiDartType(Writer w, String value) =>
-      type.convertDartTypeToFfiDartType(w, value);
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) =>
+      type.convertDartTypeToFfiDartType(w, value,
+          objCShouldRetain: objCShouldRetain);
 
   @override
   String convertFfiDartTypeToDartType(
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) =>
       type.convertFfiDartTypeToDartType(
         w,
         value,
         library,
-        isObjCOwnedReturn: isObjCOwnedReturn,
+        objCShouldRetain: objCShouldRetain,
         objCEnclosingClass: objCEnclosingClass,
       );
 
@@ -203,16 +208,21 @@ class ObjCInstanceType extends Typealias {
   }) : super._();
 
   @override
-  String convertDartTypeToFfiDartType(Writer w, String value) => '$value._id';
+  String convertDartTypeToFfiDartType(
+    Writer w,
+    String value, {
+    bool objCShouldRetain = false,
+  }) =>
+      ObjCInterface.generateGetId(value, objCShouldRetain);
 
   @override
   String convertFfiDartTypeToDartType(
     Writer w,
     String value,
     String library, {
-    bool isObjCOwnedReturn = false,
+    bool objCShouldRetain = true,
     String? objCEnclosingClass,
   }) =>
       ObjCInterface.generateConstructor(
-          objCEnclosingClass ?? 'NSObject', value, library, isObjCOwnedReturn);
+          objCEnclosingClass ?? 'NSObject', value, library, objCShouldRetain);
 }