flutter · auto-submit · Aug 17, 2022 · Aug 15, 2022 · Aug 16, 2022 · Aug 16, 2022
@@ -1,3 +1,7 @@
+## 0.9.8+4
+
+* Fixes a crash due to sending orientation change events when the engine is torn down. 
+
 ## 0.9.8+3
 
 * Fixes avoid_redundant_argument_values lint warnings and minor typos.

@@ -92,6 +92,39 @@ - (void)rotate:(UIDeviceOrientation)deviceOrientation
   [self waitForExpectationsWithTimeout:30.0 handler:nil];
 }
 
+- (void)testOrientationChanged_noRetainCycle {
+  dispatch_queue_t captureSessionQueue = dispatch_queue_create("capture_session_queue", NULL);
+  FLTCam *mockCam = OCMClassMock([FLTCam class]);
+  FLTThreadSafeMethodChannel *mockChannel = OCMClassMock([FLTThreadSafeMethodChannel class]);
+
+  __weak CameraPlugin *weakCamera;
+
+  @autoreleasepool {
+    CameraPlugin *camera = [[CameraPlugin alloc] initWithRegistry:nil messenger:nil];
+    weakCamera = camera;
+    camera.captureSessionQueue = captureSessionQueue;
+    camera.camera = mockCam;
+    camera.deviceEventMethodChannel = mockChannel;
+
+    [camera orientationChanged:
+                [self createMockNotificationForOrientation:UIDeviceOrientationLandscapeLeft]];
+  }
+
+  // Sanity check
+  XCTAssertNil(weakCamera, @"Camera must have been deallocated.");
+
+  // Must check in captureSessionQueue since orientationChanged dispatches to this queue.
+  XCTestExpectation *expectation =
+      [self expectationWithDescription:@"Dispatched to capture session queue"];
+  dispatch_async(captureSessionQueue, ^{
+    OCMVerify(never(), [mockCam setDeviceOrientation:UIDeviceOrientationLandscapeLeft]);
+    OCMVerify(never(), [mockChannel invokeMethod:@"orientation_changed" arguments:OCMOCK_ANY]);
+    [expectation fulfill];
+  });
+
+  [self waitForExpectationsWithTimeout:1 handler:nil];
+}
+
 - (NSNotification *)createMockNotificationForOrientation:(UIDeviceOrientation)deviceOrientation {
   UIDevice *mockDevice = OCMClassMock([UIDevice class]);
   OCMStub([mockDevice orientation]).andReturn(deviceOrientation);

@@ -19,7 +19,6 @@
 @interface CameraPlugin ()
 @property(readonly, nonatomic) FLTThreadSafeTextureRegistry *registry;
 @property(readonly, nonatomic) NSObject<FlutterBinaryMessenger> *messenger;
-@property(readonly, nonatomic) FLTThreadSafeMethodChannel *deviceEventMethodChannel;
 @end
 
 @implementation CameraPlugin
@@ -56,6 +55,10 @@ - (void)initDeviceEventMethodChannel {
       [[FLTThreadSafeMethodChannel alloc] initWithMethodChannel:methodChannel];
 }
 
+- (void)detachFromEngineForRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar {
+  [UIDevice.currentDevice endGeneratingDeviceOrientationNotifications];
+}
+
 - (void)startOrientationListener {
   [[UIDevice currentDevice] beginGeneratingDeviceOrientationNotifications];
   [[NSNotificationCenter defaultCenter] addObserver:self
@@ -73,11 +76,12 @@ - (void)orientationChanged:(NSNotification *)note {
     return;
   }
 
+  __weak typeof(self) weakSelf = self;
   dispatch_async(self.captureSessionQueue, ^{
     // `FLTCam::setDeviceOrientation` must be called on capture session queue.
-    [self.camera setDeviceOrientation:orientation];
+    [weakSelf.camera setDeviceOrientation:orientation];
     // `CameraPlugin::sendDeviceOrientation` can be called on any queue.
-    [self sendDeviceOrientation:orientation];
+    [weakSelf sendDeviceOrientation:orientation];
   });
 }
 
@@ -89,11 +93,11 @@ - (void)sendDeviceOrientation:(UIDeviceOrientation)orientation {
 
 - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result {
   // Invoke the plugin on another dispatch queue to avoid blocking the UI.
-  dispatch_async(_captureSessionQueue, ^{
+  __weak typeof(self) weakSelf = self;
+  dispatch_async(self.captureSessionQueue, ^{
     FLTThreadSafeFlutterResult *threadSafeResult =
         [[FLTThreadSafeFlutterResult alloc] initWithResult:result];
-
-    [self handleMethodCallAsync:call result:threadSafeResult];
+    [weakSelf handleMethodCallAsync:call result:threadSafeResult];
   });
 }
 
@@ -261,7 +265,11 @@ - (void)handleMethodCallAsync:(FlutterMethodCall *)call
 - (void)handleCreateMethodCall:(FlutterMethodCall *)call
                         result:(FLTThreadSafeFlutterResult *)result {
   // Create FLTCam only if granted camera access (and audio access if audio is enabled)
+  __weak typeof(self) weakSelf = self;
   FLTRequestCameraPermissionWithCompletionHandler(^(FlutterError *error) {
+    typeof(self) strongSelf = weakSelf;
+    if (!strongSelf) return;
+
     if (error) {
       [result sendFlutterError:error];
     } else {
@@ -272,22 +280,29 @@ - (void)handleCreateMethodCall:(FlutterMethodCall *)call
       if (audioEnabled) {
         // Setup audio capture session only if granted audio access.
         FLTRequestAudioPermissionWithCompletionHandler(^(FlutterError *error) {
+          // cannot use the outter `strongSelf`
+          typeof(self) strongSelf = weakSelf;
+          if (!strongSelf) return;
           if (error) {
             [result sendFlutterError:error];
           } else {
-            [self createCameraOnSessionQueueWithCreateMethodCall:call result:result];
+            [strongSelf createCameraOnSessionQueueWithCreateMethodCall:call result:result];
           }
         });
       } else {
-        [self createCameraOnSessionQueueWithCreateMethodCall:call result:result];
+        [strongSelf createCameraOnSessionQueueWithCreateMethodCall:call result:result];
       }
     }
   });
 }
 
 - (void)createCameraOnSessionQueueWithCreateMethodCall:(FlutterMethodCall *)createMethodCall
                                                 result:(FLTThreadSafeFlutterResult *)result {
+  __weak typeof(self) weakSelf = self;
   dispatch_async(self.captureSessionQueue, ^{
+    typeof(self) strongSelf = weakSelf;
+    if (!strongSelf) return;
+
     NSString *cameraName = createMethodCall.arguments[@"cameraName"];
     NSString *resolutionPreset = createMethodCall.arguments[@"resolutionPreset"];
     NSNumber *enableAudio = createMethodCall.arguments[@"enableAudio"];
@@ -296,22 +311,22 @@ - (void)createCameraOnSessionQueueWithCreateMethodCall:(FlutterMethodCall *)crea
                                     resolutionPreset:resolutionPreset
                                          enableAudio:[enableAudio boolValue]
                                          orientation:[[UIDevice currentDevice] orientation]
-                                 captureSessionQueue:self.captureSessionQueue
+                                 captureSessionQueue:strongSelf.captureSessionQueue
                                                error:&error];
 
     if (error) {
       [result sendError:error];
     } else {
-      if (self.camera) {
-        [self.camera close];
+      if (strongSelf.camera) {
+        [strongSelf.camera close];
       }
-      self.camera = cam;
-      [self.registry registerTexture:cam
-                          completion:^(int64_t textureId) {
-                            [result sendSuccessWithData:@{
-                              @"cameraId" : @(textureId),
-                            }];
-                          }];
+      strongSelf.camera = cam;
+      [strongSelf.registry registerTexture:cam
+                                completion:^(int64_t textureId) {
+                                  [result sendSuccessWithData:@{
+                                    @"cameraId" : @(textureId),
+                                  }];
+                                }];
     }
   });
 }

@@ -8,7 +8,7 @@
 #import "FLTCam.h"
 #import "FLTThreadSafeFlutterResult.h"
 
-/// Methods exposed for unit testing.
+/// APIs exposed for unit testing.
 @interface CameraPlugin ()
 
 /// All FLTCam's state access and capture session related operations should be on run on this queue.
@@ -17,6 +17,10 @@
 /// An internal camera object that manages camera's state and performs camera operations.
 @property(nonatomic, strong) FLTCam *camera;
 
+/// A thread safe wrapper of the method channel used to send device events such as orientation
+/// changes.
+@property(nonatomic, strong) FLTThreadSafeMethodChannel *deviceEventMethodChannel;
+
 /// Inject @p FlutterTextureRegistry and @p FlutterBinaryMessenger for unit testing.
 - (instancetype)initWithRegistry:(NSObject<FlutterTextureRegistry> *)registry
                        messenger:(NSObject<FlutterBinaryMessenger> *)messenger

@@ -20,16 +20,18 @@ - (instancetype)initWithCaptureSessionQueue:(dispatch_queue_t)captureSessionQueu
 }
 
 - (FlutterError *_Nullable)onCancelWithArguments:(id _Nullable)arguments {
+  __weak typeof(self) weakSelf = self;
   dispatch_async(self.captureSessionQueue, ^{
-    self.eventSink = nil;
+    weakSelf.eventSink = nil;
   });
   return nil;
 }
 
 - (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments
                                        eventSink:(nonnull FlutterEventSink)events {
+  __weak typeof(self) weakSelf = self;
   dispatch_async(self.captureSessionQueue, ^{
-    self.eventSink = events;
+    weakSelf.eventSink = events;
   });
   return nil;
 }
@@ -247,16 +249,18 @@ - (void)captureToFile:(FLTThreadSafeFlutterResult *)result API_AVAILABLE(ios(10)
     return;
   }
 
+  __weak typeof(self) weakSelf = self;
   FLTSavePhotoDelegate *savePhotoDelegate = [[FLTSavePhotoDelegate alloc]
            initWithPath:path
                 ioQueue:self.photoIOQueue
       completionHandler:^(NSString *_Nullable path, NSError *_Nullable error) {
-        dispatch_async(self.captureSessionQueue, ^{
-          // Dispatch back to capture session queue to delete reference.
-          // Retain cycle is broken after the dictionary entry is cleared.
-          // This is to keep the behavior with the previous `selfReference` approach in the
-          // FLTSavePhotoDelegate, where delegate is released only after capture completion.
-          [self.inProgressSavePhotoDelegates removeObjectForKey:@(settings.uniqueID)];
+        typeof(self) strongSelf = weakSelf;
+        if (!strongSelf) return;
+        dispatch_async(strongSelf.captureSessionQueue, ^{
+          // cannot use the outter `strongSelf`
+          typeof(self) strongSelf = weakSelf;
+          if (!strongSelf) return;
+          [strongSelf.inProgressSavePhotoDelegates removeObjectForKey:@(settings.uniqueID)];
         });
 
         if (error) {
@@ -387,6 +391,7 @@ - (void)captureOutput:(AVCaptureOutput *)output
     // Under rare contest scenarios, it will not block for too long since the critical section is
     // quite lightweight.
     dispatch_sync(self.pixelBufferSynchronizationQueue, ^{
+      // No need weak self because it's dispatch_sync.
       previousPixelBuffer = self.latestPixelBuffer;
       self.latestPixelBuffer = newBuffer;
     });
@@ -610,6 +615,7 @@ - (CVPixelBufferRef)copyPixelBuffer {
   __block CVPixelBufferRef pixelBuffer = nil;
   // Use `dispatch_sync` because `copyPixelBuffer` API requires synchronous return.
   dispatch_sync(self.pixelBufferSynchronizationQueue, ^{
+    // No need weak self because it's dispatch_sync.
     pixelBuffer = self.latestPixelBuffer;
     self.latestPixelBuffer = nil;
   });
@@ -917,11 +923,19 @@ - (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messen
         [[FLTThreadSafeEventChannel alloc] initWithEventChannel:eventChannel];
 
     _imageStreamHandler = imageStreamHandler;
+    __weak typeof(self) weakSelf = self;
     [threadSafeEventChannel setStreamHandler:_imageStreamHandler
                                   completion:^{
-                                    dispatch_async(self->_captureSessionQueue, ^{
-                                      self.isStreamingImages = YES;
-                                      self.streamingPendingFramesCount = 0;
+                                    typeof(self) strongSelf = weakSelf;
+                                    if (!strongSelf) return;
+
+                                    dispatch_async(strongSelf.captureSessionQueue, ^{
+                                      // cannot use the outter strongSelf
+                                      typeof(self) strongSelf = weakSelf;
+                                      if (!strongSelf) return;
+
+                                      strongSelf.isStreamingImages = YES;
+                                      strongSelf.streamingPendingFramesCount = 0;
                                     });
                                   }];
   } else {

@@ -31,13 +31,17 @@ - (void)handlePhotoCaptureResultWithError:(NSError *)error
     self.completionHandler(nil, error);
     return;
   }
+  __weak typeof(self) weakSelf = self;
   dispatch_async(self.ioQueue, ^{
+    typeof(self) strongSelf = weakSelf;
+    if (!strongSelf) return;
+
     NSData *data = photoDataProvider();
     NSError *ioError;
-    if ([data writeToFile:self.path options:NSDataWritingAtomic error:&ioError]) {
-      self.completionHandler(self.path, nil);
+    if ([data writeToFile:strongSelf.path options:NSDataWritingAtomic error:&ioError]) {
+      strongSelf.completionHandler(self.path, nil);
     } else {
-      self.completionHandler(nil, ioError);
+      strongSelf.completionHandler(nil, ioError);
     }
   });
 }

@@ -21,8 +21,11 @@ - (instancetype)initWithEventChannel:(FlutterEventChannel *)channel {
 
 - (void)setStreamHandler:(NSObject<FlutterStreamHandler> *)handler
               completion:(void (^)(void))completion {
+  __weak typeof(self) weakSelf = self;
   FLTEnsureToRunOnMainQueue(^{
-    [self.channel setStreamHandler:handler];
+    typeof(self) strongSelf = weakSelf;
+    if (!strongSelf) return;
+    [strongSelf.channel setStreamHandler:handler];
     completion();
   });
 }

@@ -52,6 +52,11 @@ - (void)sendNotImplemented {
  */
 - (void)send:(id _Nullable)result {
   FLTEnsureToRunOnMainQueue(^{
+    // WARNING: Should not use weak self, because `FlutterResult`s are passed as arguments
+    // (retained within call stack, but not in the heap). FLTEnsureToRunOnMainQueue may trigger a
+    // context switch (when calling from background thread), in which case using weak self will
+    // always result in a nil self. Alternative to using strong self, we can also create a local
+    // strong variable to be captured by this block.
     self.flutterResult(result);
   });
 }

@@ -20,8 +20,9 @@ - (instancetype)initWithMethodChannel:(FlutterMethodChannel *)channel {
 }
 
 - (void)invokeMethod:(NSString *)method arguments:(id)arguments {
+  __weak typeof(self) weakSelf = self;
   FLTEnsureToRunOnMainQueue(^{
-    [self.channel invokeMethod:method arguments:arguments];
+    [weakSelf.channel invokeMethod:method arguments:arguments];
   });
 }
 

@@ -21,20 +21,25 @@ - (instancetype)initWithTextureRegistry:(NSObject<FlutterTextureRegistry> *)regi
 
 - (void)registerTexture:(NSObject<FlutterTexture> *)texture
              completion:(void (^)(int64_t))completion {
+  __weak typeof(self) weakSelf = self;
   FLTEnsureToRunOnMainQueue(^{
-    completion([self.registry registerTexture:texture]);
+    typeof(self) strongSelf = weakSelf;
+    if (!strongSelf) return;
+    completion([strongSelf.registry registerTexture:texture]);
   });
 }
 
 - (void)textureFrameAvailable:(int64_t)textureId {
+  __weak typeof(self) weakSelf = self;
   FLTEnsureToRunOnMainQueue(^{
-    [self.registry textureFrameAvailable:textureId];
+    [weakSelf.registry textureFrameAvailable:textureId];
   });
 }
 
 - (void)unregisterTexture:(int64_t)textureId {
+  __weak typeof(self) weakSelf = self;
   FLTEnsureToRunOnMainQueue(^{
-    [self.registry unregisterTexture:textureId];
+    [weakSelf.registry unregisterTexture:textureId];
   });
 }
 

@@ -2,7 +2,7 @@ name: camera_avfoundation
 description: iOS implementation of the camera plugin.
 repository: https://github.com/flutter/plugins/tree/main/packages/camera/camera_avfoundation
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
-version: 0.9.8+3
+version: 0.9.8+4
 
 environment:
   sdk: ">=2.14.0 <3.0.0"