fix: Fix runAsync not properly retaining/releasing Frame (#2829)

mrousavy · May 1, 2024 · e75bb9f · e75bb9f
1 parent 96c2ecf
commit e75bb9f
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 22 deletions.
diff --git a/package/ios/FrameProcessors/Frame.h b/package/ios/FrameProcessors/Frame.h
@@ -19,6 +19,9 @@ NS_ASSUME_NONNULL_BEGIN
 - (instancetype)initWithBuffer:(CMSampleBufferRef)buffer orientation:(UIImageOrientation)orientation;
 - (instancetype)init NS_UNAVAILABLE;
 
+- (void)incrementRefCount;
+- (void)decrementRefCount;
+
 @property(nonatomic, readonly) CMSampleBufferRef buffer;
 @property(nonatomic, readonly) UIImageOrientation orientation;
 

diff --git a/package/ios/FrameProcessors/Frame.m b/package/ios/FrameProcessors/Frame.m
@@ -20,12 +20,15 @@ - (instancetype)initWithBuffer:(CMSampleBufferRef)buffer orientation:(UIImageOri
   if (self) {
     _buffer = buffer;
     _orientation = orientation;
-    CFRetain(buffer);
   }
   return self;
 }
 
-- (void)dealloc {
+- (void)incrementRefCount {
+  CFRetain(_buffer);
+}
+
+- (void)decrementRefCount {
   CFRelease(_buffer);
 }
 

diff --git a/package/ios/FrameProcessors/FrameHostObject.h b/package/ios/FrameProcessors/FrameHostObject.h
@@ -10,7 +10,6 @@
 
 #import <CoreMedia/CMSampleBuffer.h>
 #import <jsi/jsi.h>
-#import <mutex>
 
 #import "Frame.h"
 
@@ -29,8 +28,4 @@ class JSI_EXPORT FrameHostObject : public jsi::HostObject {
 
 private:
   Frame* getFrame();
-
-private:
-  std::mutex _mutex;
-  size_t _refCount = 0;
 };
diff --git a/package/ios/FrameProcessors/FrameHostObject.mm b/package/ios/FrameProcessors/FrameHostObject.mm
@@ -40,8 +40,7 @@
 }
 
 Frame* FrameHostObject::getFrame() {
-  Frame* frame = this->frame;
-  if (frame == nil || !frame.isValid) {
+  if (!frame.isValid) [[unlikely]] {
     throw std::runtime_error("Frame is already closed! "
                              "Are you trying to access the Image data outside of a Frame Processor's lifetime?\n"
                              "- If you want to use `console.log(frame)`, use `console.log(frame.toString())` instead.\n"
@@ -99,25 +98,14 @@
   // Internal methods
   if (name == "incrementRefCount") {
     auto incrementRefCount = JSI_FUNC {
-      std::lock_guard lock(this->_mutex);
-
-      // Increment our self-counted ref count by one.
-      _refCount++;
+      [frame incrementRefCount];
       return jsi::Value::undefined();
     };
     return jsi::Function::createFromHostFunction(runtime, jsi::PropNameID::forUtf8(runtime, "incrementRefCount"), 0, incrementRefCount);
   }
   if (name == "decrementRefCount") {
     auto decrementRefCount = JSI_FUNC {
-      std::lock_guard lock(this->_mutex);
-
-      // Decrement our self-counted ref count by one.
-      _refCount--;
-      if (_refCount < 1) {
-        // ARC will then delete the Frame and the underlying Frame Buffer.
-        this->frame = nil;
-      }
-
+      [frame decrementRefCount];
       return jsi::Value::undefined();
     };
     return jsi::Function::createFromHostFunction(runtime, jsi::PropNameID::forUtf8(runtime, "decrementRefCount"), 0, decrementRefCount);