CGImageRef gets released before the listener is called

[kekland]

Hi!

I'm trying to use the AVAssetImageGenerator.generateCGImagesAsynchronouslyForTimes_completionHandler via ffigen to extract video frames (e.g. the thumbnail). For some reason, when the completion handler is called, in some situations (not always!) it seems like the CGImageRef is already released when the listener is called. Here's a shortened piece of code that shows the usage from Dart:

  Future<List<ui.Image>> _extractFramesAt(
    VideoSource source, {
    required List<Duration> timestamps,
    ui.Size? preferredSize,
  }) async {
    return using<Future<List<ui.Image>>>((alloc) async {
      // Prepare the asset image generator
      final avAsset = videoSourceToAVAsset(source);
      final generator = AVAssetImageGenerator.assetImageGeneratorWithAsset_(avAsset);
      generator.appliesPreferredTrackTransform = true;

      // Omitted...

      // This closure is called every time an image is generated
      final closure = ObjCBlock_ffiVoid_CMTime_CGImage_CMTime_AVAssetImageGeneratorResult_NSError.listener(
        (requestedTime, cgImagePointer, actualTime, result, error) {
          if (error != null) {
            imageStream.addError(error);
            return;
          }

          if (cgImagePointer == nullptr) {
            imageStream.addError(Exception('No image generated'));
            return;
          }

          // Paint the image onto a CGBitmap
          final width = darwinLib.CGImageGetWidth(cgImagePointer);
          final height = darwinLib.CGImageGetHeight(cgImagePointer);
          final colorSpace = darwinLib.CGColorSpaceCreateDeviceRGB();

          if (width == 0 || height == 0) { // <- This gets called often with both width and height equal to 0.
            imageStream.addError(Exception('Invalid image size: $width x $height'));
            return;
          }

          // Omitted...
        },
      );

      // Run the generator. The closure will be called for each timestamp
      generator.generateCGImagesAsynchronouslyForTimes_completionHandler_(
        cmTimeArray,
        closure,
      );
      
      // Omitted...
    });
  }

From what I can understand, somehow the CGImageRef is getting released too early, before the listener is called. I was able to consistently replicate this on my phone. Also, I was able to fix this in the generated code by adding an explicit CGImageRetain call to the listener block:

// Before
typedef void  (^ListenerBlock51)(CMTime , struct CGImage * , CMTime , AVAssetImageGeneratorResult , NSError* );
ListenerBlock51 wrapListenerBlock_ObjCBlock_ffiVoid_CMTime_CGImage_CMTime_AVAssetImageGeneratorResult_NSError(ListenerBlock51 block) NS_RETURNS_RETAINED {
  return ^void(CMTime arg0, struct CGImage * arg1, CMTime arg2, AVAssetImageGeneratorResult arg3, NSError* arg4) {
    block(arg0, arg1, arg2, arg3, objc_retain(arg4));
  };
}

// After
typedef void  (^ListenerBlock51)(CMTime , struct CGImage * , CMTime , AVAssetImageGeneratorResult , NSError* );
ListenerBlock51 wrapListenerBlock_ObjCBlock_ffiVoid_CMTime_CGImage_CMTime_AVAssetImageGeneratorResult_NSError(ListenerBlock51 block) NS_RETURNS_RETAINED {
  return ^void(CMTime arg0, struct CGImage * arg1, CMTime arg2, AVAssetImageGeneratorResult arg3, NSError* arg4) {
    CGImageRetain(arg1); // <- Added this
    block(arg0, arg1, arg2, arg3, objc_retain(arg4));
  };
}

and then releasing the CGImage on the Dart side. This seems to fix the issue, but I'm not sure whether it should be that way.

If needed, I can provide an example project which should be able to reproduce this issue. I'm using ffigen/objc from #1463

[kekland]

https://stackoverflow.com/questions/20530342/correct-way-to-handle-cgimageref-in-containers-under-arc this seems to be relevant here (and probably for other CF types)

[liamappelbe]

Looks like CGImageRef is a struct, so its ref counting is not handled in the usual way with objc_retain/objc_release, and is instead handled by CGImageRetain/CGImageRelease. I don't think there's a way for us to automatically insert these retains and releases because this function is specific to this struct type. This is probably one area where there will always need to be user written native code.

@kekland The workaround you've written there is probably the best approach. The only improvement I'd make is to try pulling wrapListenerBlock_ObjCBlock_ffiVoid_CMTime_CGImage_CMTime_AVAssetImageGeneratorResult_NSError out into its own source file (and maybe giving it a shorter name 😉) so that your fix doesn't get nuked every time you regenerate the bindings. You'll then need to manually pass your Dart block through the wrapping function before using it (final retainingBlock = wrapListenerBlock_...(dartBlock);).

[kekland]

@liamappelbe thanks for the reply, will do that.

About handling non-objc pointers with ARC - it seems like there's methods CFBridgingRelease/CFBridgingRetain that's designed to make this (and other CF types) compatible with ARC (CFBridgingRelease transfers the ownership of the object to ARC). In the documentation for CGImageRetain (or release) they mention that its behavior is the same as CFRetain, but with an added nullptr check, so it doesn't crash in this case.

I think it's possible to use CFBridgingRetain/CFBridgingRelease with CF objects, by retaining them when the listener is executed on platform thread, and releasing once the listener is completed on Dart. The only question is how to detect the types where this is required?

I will probably take a deeper look into this today, will post my findings here.

[liamappelbe]

Hmmm. I'm a bit confused by the fact that CGImageRef is a struct, not a pointer to a struct/object. So I wonder if it's literally possible to pass a CGImageRef to CFRetain (which expects a CFTypeRef, which is just a void*), or if the documentation is being hand-wavy there.

[kekland]

CGImageRef is defined as the following:

// CoreGraphics/CGImage.h:12
typedef struct CF_BRIDGED_TYPE(id) CGImage *CGImageRef;  /* Swift Sendable */

CFTypeRef:

// CoreFoundation/CFBase.h:497
typedef const CF_BRIDGED_TYPE(id) void * CFTypeRef;

I think it should be possible. Calling CFRetain(arg1) leads to no warnings or errors.

Reading through https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html - it seems that the safe approach would be to not pass it over to ARC (as by their definition if you don't create the CF object via a method called Create or Copy you do not own the object), but rather add a CFRetain on the generated obj-c listener block, and then to balance, add CFRelease on the generated Dart code, after the listener is completed. It should also have a null pointer check, as CFRetain/CFRelease crashes when a null pointer is passed.

I'm not sure whether that would be in the scope of the ffigen/obj-c package, but it would probably make sense that this is also automatically managed

[liamappelbe]

Sounds good. The only question is how to detect these types from libclang

[kekland]

This is where I have absolutely zero knowledge :P My only idea was to hardcode a list of CF types. All CF types have a method {$type}GetTypeID, like CFBooleanGetTypeID(), CGImageGetTypeID(), etc, to be used like follows:

https://developer.apple.com/documentation/corefoundation/cftypeid

      if (CFGetTypeID(arg1) == CGImageGetTypeID()) {
        // This is a CGImage!
      }

I wrote a Python script to go through the headers in /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/ and extract all of the CF types:

import pathlib

headers_location = pathlib.Path('/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/')

# recursively find all headers in the SDK
headers = list(headers_location.glob('**/*.h'))

cfTypesList = []
for header in headers:
  try:
    with open(header, 'r', encoding='latin-1') as f:
      lines = f.readlines()

      for line in lines:
        # If line has `CFTypeID {x}GetTypeID(void)`
        if 'CFTypeID' in line and 'GetTypeID' in line and 'void' in line:
          # print(header)
          # print(line)

          # Extract the type name
          tokens = line.split()

          # Extract the type name
          type_name = tokens[tokens.index('CFTypeID') + 1].split('GetTypeID')[0]
          cfTypesList.append(type_name)

  except Exception as e:
    print('Error reading file:', header)
    print(e)
    continue

print(cfTypesList)

which results in the following:

['DRErase', 'DRNotificationCenter', 'DRFolder', 'DRBurn', 'DRTrack', 'DRFile', 'DRDevice', 'CGFunction', 'CGEvent', 'CGDataProvider', 'CGLayer', 'CGPDFPage', 'CGPath', 'CGColor', 'CGPSConverter', 'CGPDFDocument', 'CGFont', 'CGShading', 'CGColorConversionInfo', 'CGDisplayMode', 'CGColorSpace', 'CGDisplayStreamUpdate', 'CGDisplayStream', 'CGGradient', 'CGContext', 'CGDataConsumer', 'CGEventSource', 'CGPattern', 'CGImage', 'MTAudioProcessingTap', 'CVOpenGLTextureCache', 'CVPixelBufferPool', 'CVOpenGLTexture', 'CVPixelBuffer', 'CVOpenGLBufferPool', 'CVOpenGLBuffer', 'CVMetalTexture', 'CVDisplayLink', 'CVMetalTextureCache', 'AXUIElement', 'AXTextMarker', 'AXTextMarkerRange', 'AXObserver', 'CTRunDelegate', 'CTTextTab', 'CTFontDescriptor', 'CTFont', 'CTLine', 'CTFramesetter', 'CTRubyAnnotation', 'CTTypesetter', 'CTFrame', 'CTRun', 'CTParagraphStyle', 'CTFontCollection', 'CTGlyphInfo', 'IOHIDEventSystemClient', 'IOHIDServiceClient', 'IOHIDUserDevice', 'IOHIDDevice', 'IOHIDManager', 'IOHIDQueue', 'IOHIDTransaction', 'IOHIDValue', 'IOHIDElement', 'CGImageMetadata', 'CGImageMetadataTag', 'CGImageSource', 'CGImageDestination', 'SecDigestTransform', 'SecACL', 'CMSDecoder', 'SecKeychainItem', 'SecKeychain', 'SecCode', 'SecIdentitySearch', 'SecCertificate', 'SecKey', 'SecTask', 'SecTrustedApplication', 'SecTrust', 'SecPolicy', 'SecRequirement', 'SecAccessControl', 'SecIdentity', 'SecKeychainSearch', 'SecTransform', 'SecGroupTransform', 'SecPolicySearch', 'SecAccess', 'CMSEncoder', 'SecStaticCode', 'SecDecryptTransform', 'SecEncryptTransform', 'IOSurface', 'CMTaggedBufferGroup', 'CMSimpleQueue', 'CMFormatDescription', 'CMBlockBuffer', 'CMBufferQueue', 'CMTagCollection', 'CMMemoryPool', 'CMSampleBuffer', 'ODNode', 'ODSession', 'ODContext', 'ODRecord', 'ODQuery', 'ColorSyncCMM', 'ColorSyncTransform', 'ColorSyncProfile', 'MDQuery', 'MDLabel', 'MDItem', 'FSFileOperation', 'FSFileSecurity', 'QLThumbnail', 'QLThumbnailRequest', 'QLPreviewRequest', 'CFUUID', 'CFBundle', 'CFBag', 'CFNotificationCenter', 'CFUserNotification', 'CFXMLNode', 'CFArray', 'CFReadStream', 'CFWriteStream', 'CFDictionary', 'CFFileSecurity', 'CFCharacterSet', 'CFDateFormatter', 'CFTree', 'CFMessagePort', 'CFURLEnumerator', 'CFMachPort', 'CFPlugIn', 'CFPlugInInstance', 'CFDate', 'CFStringTokenizer', 'CFLocale', 'CFData', 'CFNumberFormatter', 'CFTimeZone', 'CFError', 'CFBoolean', 'CFNumber', 'CFSocket', 'CFFileDescriptor', 'CFURL', 'CFBitVector', 'CFXMLParser', 'CFAttributedString', 'CFRunLoop', 'CFRunLoopSource', 'CFRunLoopObserver', 'CFRunLoopTimer', 'CFString', 'CFBinaryHeap', 'CFNull', 'CFAllocator', 'CFSet', 'CFCalendar', 'LSMMap', 'LSMText', 'LSMResult', 'DASession', 'DAApprovalSession', 'DADisk', 'GLKMatrixStack', 'VTMultiPassStorage', 'VTFrameSilo']

When generating the listeners, we can check if the type of the reference matches one of those types in the list, and in that case use the CF retain/release mechanisms