[camera]use weak self to fix a crash due to orientation change (#6277)
diff --git a/packages/camera/camera_avfoundation/CHANGELOG.md b/packages/camera/camera_avfoundation/CHANGELOG.md
index 2726309..4df497b 100644
--- a/packages/camera/camera_avfoundation/CHANGELOG.md
+++ b/packages/camera/camera_avfoundation/CHANGELOG.md
@@ -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.
diff --git a/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraOrientationTests.m b/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraOrientationTests.m
index 29fc325..60e88ff 100644
--- a/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraOrientationTests.m
+++ b/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraOrientationTests.m
@@ -92,6 +92,39 @@
[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);
diff --git a/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m b/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m
index cb19c09..628211a 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m
+++ b/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m
@@ -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 @@
[[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 @@
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)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)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,14 +280,17 @@
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];
}
}
});
@@ -287,7 +298,11 @@
- (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 @@
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),
+ }];
+ }];
}
});
}
diff --git a/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin_Test.h b/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin_Test.h
index 77a758d..f6c97da 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin_Test.h
+++ b/packages/camera/camera_avfoundation/ios/Classes/CameraPlugin_Test.h
@@ -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
diff --git a/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m b/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m
index f267604..90b81ad 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m
+++ b/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m
@@ -20,16 +20,18 @@
}
- (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 @@
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 @@
// 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 @@
__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 @@
[[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 {
diff --git a/packages/camera/camera_avfoundation/ios/Classes/FLTSavePhotoDelegate.m b/packages/camera/camera_avfoundation/ios/Classes/FLTSavePhotoDelegate.m
index 1df1708..617890c 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/FLTSavePhotoDelegate.m
+++ b/packages/camera/camera_avfoundation/ios/Classes/FLTSavePhotoDelegate.m
@@ -31,13 +31,17 @@
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);
}
});
}
diff --git a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeEventChannel.m b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeEventChannel.m
index 46941bb..ed749a3 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeEventChannel.m
+++ b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeEventChannel.m
@@ -21,8 +21,11 @@
- (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();
});
}
diff --git a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeFlutterResult.m b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeFlutterResult.m
index ad125f7..283a0d6 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeFlutterResult.m
+++ b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeFlutterResult.m
@@ -52,6 +52,11 @@
*/
- (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);
});
}
diff --git a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeMethodChannel.m b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeMethodChannel.m
index 5b29b70..df7c169 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeMethodChannel.m
+++ b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeMethodChannel.m
@@ -20,8 +20,9 @@
}
- (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];
});
}
diff --git a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeTextureRegistry.m b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeTextureRegistry.m
index 349b89f..b82d566 100644
--- a/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeTextureRegistry.m
+++ b/packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeTextureRegistry.m
@@ -21,20 +21,25 @@
- (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];
});
}
diff --git a/packages/camera/camera_avfoundation/pubspec.yaml b/packages/camera/camera_avfoundation/pubspec.yaml
index c2b9dcb..3d1b7c6 100644
--- a/packages/camera/camera_avfoundation/pubspec.yaml
+++ b/packages/camera/camera_avfoundation/pubspec.yaml
@@ -2,7 +2,7 @@
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"
