8000 [camera]use weak self to fix a crash due to orientation change (#6277) · flutter/plugins@fbdf7e3 · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit fbdf7e3

Browse files
hellohuanlinhellohuanlin
authored
[camera]use weak self to fix a crash due to orientation change (#6277)
1 parent c816759 commit fbdf7e3

File tree

11 files changed

+127
-39
lines changed

11 files changed

+127
-39
lines changed

‎packages/camera/camera_avfoundation/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.9.8+4
2+
3+
* Fixes a crash due to sending orientation change events when the engine is torn down.
4+
15
## 0.9.8+3
26

37
* Fixes avoid_redundant_argument_values lint warnings and minor typos.

‎packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraOrientationTests.m

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,39 @@ - (void)rotate:(UIDeviceOrientation)deviceOrientation
9292
[self waitForExpectationsWithTimeout:30.0 handler:nil];
9393
}
9494

95+
- (void)testOrientationChanged_noRetainCycle {
96+
dispatch_queue_t captureSessionQueue = dispatch_queue_create("capture_session_queue", NULL);
97+
FLTCam *mockCam = OCMClassMock([FLTCam class]);
98+
FLTThreadSafeMethodChannel *mockChannel = OCMClassMock([FLTThreadSafeMethodChannel class]);
99+
100+
__weak CameraPlugin *weakCamera;
101+
102+
@autoreleasepool {
103+
CameraPlugin *camera = [[CameraPlugin alloc] initWithRegistry:nil messenger:nil];
104+
weakCamera = camera;
105+
camera.captureSessionQueue = captureSessionQueue;
106+
camera.camera = mockCam;
107+
camera.deviceEventMethodChannel = mockChannel;
108+
109+
[camera orientationChanged:
110+
[self createMockNotificationForOrientation:UIDeviceOrientationLandscapeLeft]];
111+
}
112+
113+
// Sanity check
114+
XCTAssertNil(weakCamera, @"Camera must have been deallocated.");
115+
116+
// Must check in captureSessionQueue since orientationChanged dispatches to this queue.
117+
XCTestExpectation *expectation =
118+
[self expectationWithDescription:@"Dispatched to capture session queue"];
119+
dispatch_async(captureSessionQueue, ^{
120+
OCMVerify(never(), [mockCam setDeviceOrientation:UIDeviceOrientationLandscapeLeft]);
121+
OCMVerify(never(), [mockChannel invokeMethod:@"orientation_changed" arguments:OCMOCK_ANY]);
122+
[expectation fulfill];
123+
});
124+
125+
[self waitForExpectationsWithTimeout:1 handler:nil];
126+
}
127+
95128
- (NSNotification *)createMockNotificationForOrientation:(UIDeviceOrientation)deviceOrientation {
96129
UIDevice *mockDevice = OCMClassMock([UIDevice class]);
97130
OCMStub([mockDevice orientation]).andReturn(deviceOrientation);

‎packages/camera/camera_avfoundation/ios/Classes/CameraPlugin.m

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
@interface CameraPlugin ()
2020
@property(readonly, nonatomic) FLTThreadSafeTextureRegistry *registry;
2121
@property(readonly, nonatomic) NSObject<FlutterBinaryMessenger> *messenger;
22-
@property(readonly, nonatomic) FLTThreadSafeMethodChannel *deviceEventMethodChannel;
2322
@end
2423

2524
@implementation CameraPlugin
@@ -56,6 +55,10 @@ - (void)initDeviceEventMethodChannel {
5655
[[FLTThreadSafeMethodChannel alloc] initWithMethodChannel:methodChannel];
5756
}
5857

58+
- (void)detachFromEngineForRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar {
59+
[UIDevice.currentDevice endGeneratingDeviceOrientationNotifications];
60+
}
61+
5962
- (void)startOrientationListener {
6063
[[UIDevice currentDevice] beginGeneratingDeviceOrientationNotifications];
6164
[[NSNotificationCenter defaultCenter] addObserver:self
@@ -73,11 +76,12 @@ - (void)orientationChanged:(NSNotification *)note {
7376
return;
7477
}
7578

79+
__weak typeof(self) weakSelf = self;
7680
dispatch_async(self.captureSessionQueue, ^{
7781
// `FLTCam::setDeviceOrientation` must be called on capture session queue.
78-
[self.camera setDeviceOrientation:orientation];
82+
[weakSelf.camera setDeviceOrientation:orientation];
7983
// `CameraPlugin::sendDeviceOrientation` can be called on any queue.
80-
[self sendDeviceOrientation:orientation];
84+
[weakSelf sendDeviceOrientation:orientation];
8185
});
8286
}
8387

@@ -89,11 +93,11 @@ - (void)sendDeviceOrientation:(UIDeviceOrientation)orientation {
8993

9094
- (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result {
9195
// Invoke the plugin on another dispatch queue to avoid blocking the UI.
92-
dispatch_async(_captureSessionQueue, ^{
96+
__weak typeof(self) weakSelf = self;
97+
dispatch_async(self.captureSessionQueue, ^{
9398
FLTThreadSafeFlutterResult *threadSafeResult =
9499
[[FLTThreadSafeFlutterResult alloc] initWithResult:result];
95-
96-
[self handleMethodCallAsync:call result:threadSafeResult];
100+
[weakSelf handleMethodCallAsync:call result:threadSafeResult];
97101
});
98102
}
99103

@@ -261,7 +265,11 @@ - (void)handleMethodCallAsync:(FlutterMethodCall *)call
261265
- (void)handleCreateMethodCall:(FlutterMethodCall *)call
262266
result:(FLTThreadSafeFlutterResult *)result {
263267
// Create FLTCam only if granted camera access (and audio access if audio is enabled)
268+
__weak typeof(self) weakSelf = self;
264269
FLTRequestCameraPermissionWithCompletionHandler(^(FlutterError *error) {
270+
typeof(self) strongSelf = weakSelf;
271+
if (!strongSelf) return;
272+
265273
if (error) {
266274
[result sendFlutterError:error];
267275
} else {
@@ -272,22 +280,29 @@ - (void)handleCreateMethodCall:(FlutterMethodCall *)call
272280
if (audioEnabled) {
273281
// Setup audio capture session only if granted audio access.
274282
FLTRequestAudioPermissionWithCompletionHandler(^(FlutterError *error) {
283+
// cannot use the outter `strongSelf`
284+
typeof(self) strongSelf = weakSelf;
285+
if (!strongSelf) return;
275286
if (error) {
276287
[result sendFlutterError:error];
277288
} else {
278-
[self createCameraOnSessionQueueWithCreateMethodCall:call result:result];
289+
[strongSelf createCameraOnSessionQueueWithCreateMethodCall:call result:result];
279290
}
280291
});
281292
} else {
282-
[self createCameraOnSessionQueueWithCreateMethodCall:call result:result];
293+
[strongSelf createCameraOnSessionQueueWithCreateMethodCall:call result:result];
283294
}
284295
}
285296
});
286297
}
287298

288299
- (void)createCameraOnSessionQueueWithCreateMethodCall:(FlutterMethodCall *)createMethodCall
289300
result:(FLTThreadSafeFlutterResult *)result {
301+
__weak typeof(self) weakSelf = self;
290302
dispatch_async(self.captureSessionQueue, ^{
303+
typeof(self) strongSelf = weakSelf;
304+
if (!strongSelf) return;
305+
291306
NSString *cameraName = createMethodCall.arguments[@"cameraName"];
292307
NSString *resolutionPreset = createMethodCall.arguments[@"resolutionPreset"];
293308
NSNumber *enableAudio = createMethodCall.arguments[@"enableAudio"];
@@ -296,22 +311,22 @@ - (void)createCameraOnSessionQueueWithCreateMethodCall:(FlutterMethodCall *)crea
296311
resolutionPreset:resolutionPreset
297312
enableAudio:[enableAudio boolValue]
298313
orientation:[[UIDevice currentDevice] orientation]
299-
captureSessionQueue:self.captureSessionQueue
314+
captureSessionQueue:strongSelf.captureSessionQueue
300315
error:&error];
301316

302317
if (error) {
303318
[result sendError:error];
304319
} else {
305-
if (self.camera) {
306-
[self.camera close];
320+
if (strongSelf.camera) {
321+
[strongSelf.camera close];
307322
}
308-
self.camera = cam;
309-
[self.registry registerTexture:cam
310-
completion:^(int64_t textureId) {
311-
[result sendSuccessWithData:@{
312-
@"cameraId" : @(textureId),
313-
}];
314-
}];
323+
strongSelf.camera = cam;
324+
[strongSelf.registry registerTexture:cam
325+
completion:^(int64_t textureId) {
326+
[result sendSuccessWithData:@{
327+
@"cameraId" : @(textureId),
328+
}];
329+
}];
315330
}
316331
});
317332
}

‎packages/camera/camera_avfoundation/ios/Classes/CameraPlugin_Test.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#import "FLTCam.h"
99
#import "FLTThreadSafeFlutterResult.h"
1010

11-
/// Methods exposed for unit testing.
11+
/// APIs exposed for unit testing.
1212
@interface CameraPlugin ()
1313

1414
/// All FLTCam's state access and capture session related operations should be on run on this queue.
@@ -17,6 +17,10 @@
1717
/// An internal camera object that manages camera's state and performs camera operations.
1818
@property(nonatomic, strong) FLTCam *camera;
1919

20+
/// A thread safe wrapper of the method channel used to send device events such as orientation
21+
/// changes.
22+
@property(nonatomic, strong) FLTThreadSafeMethodChannel *deviceEventMethodChannel;
23+
2024
/// Inject @p FlutterTextureRegistry and @p FlutterBinaryMessenger for unit testing.
2125
- (instancetype)initWithRegistry:(NSObject<FlutterTextureRegistry> *)registry
2226
messenger:(NSObject<FlutterBinaryMessenger> *)messenger

‎packages/camera/camera_avfoundation/ios/Classes/FLTCam.m

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,18 @@ - (instancetype)initWithCaptureSessionQueue:(dispatch_queue_t)captureSessionQueu
2020
}
2121

2222
- (FlutterError *_Nullable)onCancelWithArguments:(id _Nullable)arguments {
23+
__weak typeof(self) weakSelf = self;
2324
dispatch_async(self.captureSessionQueue, ^{
24-
self.eventSink = nil;
25+
weakSelf.eventSink = nil;
2526
});
2627
return nil;
2728
}
2829

2930
- (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments
3031
eventSink:(nonnull FlutterEventSink)events {
32+
__weak typeof(self) weakSelf = self;
3133
dispatch_async(self.captureSessionQueue, ^{
32-
self.eventSink = events;
34+
weakSelf.eventSink = events;
3335
});
3436
return nil;
3537
}
@@ -247,16 +249,18 @@ - (void)captureToFile:(FLTThreadSafeFlutterResult *)result API_AVAILABLE(ios(10)
247249
return;
248250
}
249251

252+
__weak typeof(self) weakSelf = self;
250253
FLTSavePhotoDelegate *savePhotoDelegate = [[FLTSavePhotoDelegate alloc]
251254
initWithPath:path
252255
ioQueue:self.photoIOQueue
253256
completionHandler:^(NSString *_Nullable path, NSError *_Nullable error) {
254-
dispatch_async(self.captureSessionQueue, ^{
255-
// Dispatch back to capture session queue to delete reference.
256-
// Retain cycle is broken after the dictionary entry is cleared.
257-
// This is to keep the behavior with the previous `selfReference` approach in the
258-
// FLTSavePhotoDelegate, where delegate is released only after capture completion.
259-
[self.inProgressSavePhotoDelegates removeObjectForKey:@(settings.uniqueID)];
257+
typeof(self) strongSelf = weakSelf;
258+
if (!strongSelf) return;
259+
dispatch_async(strongSelf.captureSessionQueue, ^{
260+
// cannot use the outter `strongSelf`
261+
typeof(self) strongSelf = weakSelf;
262+
if (!strongSelf) return;
263+
[strongSelf.inProgressSavePhotoDelegates removeObjectForKey:@(settings.uniqueID)];
260264
});
261265

262266
if (error) {
@@ -387,6 +391,7 @@ - (void)captureOutput:(AVCaptureOutput *)output
387391
// Under rare contest scenarios, it will not block for too long since the critical section is
388392
// quite lightweight.
389393
dispatch_sync(self.pixelBufferSynchronizationQueue, ^{
394+
// No need weak self because it's dispatch_sync.
390395
previousPixelBuffer = self.latestPixelBuffer;
391396
self.latestPixelBuffer = newBuffer;
392397
});
@@ -610,6 +615,7 @@ - (CVPixelBufferRef)copyPixelBuffer {
610615
__block CVPixelBufferRef pixelBuffer = nil;
611616
// Use `dispatch_sync` because `copyPixelBuffer` API requires synchronous return.
612617
dispatch_sync(self.pixelBufferSynchronizationQueue, ^{
618+
// No need weak self because it's dispatch_sync.
613619
pixelBuffer = self.latestPixelBuffer;
614620
self.latestPixelBuffer = nil;
615621
});
@@ -917,11 +923,19 @@ - (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messen
917923
[[FLTThreadSafeEventChannel alloc] initWithEventChannel:eventChannel];
918924

919925
_imageStreamHandler = imageStreamHandler;
926+
__weak typeof(self) weakSelf = self;
920927
[threadSafeEventChannel setStreamHandler:_imageStreamHandler
921928
completion:^{
922-
dispatch_async(self->_captureSessionQueue, ^{
923-
self.isStreamingImages = YES;
924-
self.streamingPendingFramesCount = 0;
929+
typeof(self) strongSelf = weakSelf;
930+
if (!strongSelf) return;
931+
932+
dispatch_async(strongSelf.captureSessionQueue, ^{
933+
// cannot use the outter strongSelf
934+
typeof(self) strongSelf = weakSelf;
935+
if (!strongSelf) return;
936+
937+
strongSelf.isStreamingImages = YES;
938+
strongSelf.streamingPendingFramesCount = 0;
925939
});
926940
}];
927941
} else {

‎packages/camera/camera_avfoundation/ios/Classes/FLTSavePhotoDelegate.m

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,17 @@ - (void)handlePhotoCaptureResultWithError:(NSError *)error
3131
self.completionHandler(nil, error);
3232
return;
3333
}
34+
__weak typeof(self) weakSelf = self;
3435
dispatch_async(self.ioQueue, ^{
36+
typeof(self) strongSelf = weakSelf;
37+
if (!strongSelf) return;
38+
3539
NSData *data = photoDataProvider();
3640
NSError *ioError;
37-
if ([data writeToFile:self.path options:NSDataWritingAtomic error:&ioError]) {
38-
self.completionHandler(self.path, nil);
41+
if ([data writeToFile:strongSelf.path options:NSDataWritingAtomic error:&ioError]) {
42+
strongSelf.completionHandler(self.path, nil);
3943
} else {
40-
self.completionHandler(nil, ioError);
44+
strongSelf.completionHandler(nil, ioError);
4145
}
4246
});
4347
}

‎packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeEventChannel.m

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@ - (instancetype)initWithEventChannel:(FlutterEventChannel *)channel {
2121

2222
- (void)setStreamHandler:(NSObject<FlutterStreamHandler> *)handler
2323
completion:(void (^)(void))completion {
24+
__weak typeof(self) weakSelf = self;
2425
FLTEnsureToRunOnMainQueue(^{
25-
[self.channel setStreamHandler:handler];
26+
typeof(self) strongSelf = weakSelf;
27+
if (!strongSelf) return;
28+
[strongSelf.channel setStreamHandler:handler];
2629
completion();
2730
});
2831
}

‎packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeFlutterResult.m

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ - (void)sendNotImplemented {
5252
*/
5353
- (void)send:(id _Nullable)result {
5454
FLTEnsureToRunOnMainQueue(^{
55+
// WARNING: Should not use weak self, because `FlutterResult`s are passed as arguments
56+
// (retained within call stack, but not in the heap). FLTEnsureToRunOnMainQueue may trigger a
57+
// context switch (when calling from background thread), in which case using weak self will
58+
// always result in a nil self. Alternative to using strong self, we can also create a local
59+
// strong variable to be captured by this block.
5560
self.flutterResult(result);
5661
});
5762
}

‎packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeMethodChannel.m

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ - (instancetype)initWithMethodChannel:(FlutterMethodChannel *)channel {
2020
}
2121

2222
- (void)invokeMethod:(NSString *)method arguments:(id)arguments {
23+
__weak typeof(self) weakSelf = self;
2324
FLTEnsureToRunOnMainQueue(^{
24-
[self.channel invokeMethod:method arguments:arguments];
25+
[weakSelf.channel invokeMethod:method arguments:arguments];
2526
});
2627
}
2728

‎packages/camera/camera_avfoundation/ios/Classes/FLTThreadSafeTextureRegistry.m

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,25 @@ - (instancetype)initWithTextureRegistry:(NSObject<FlutterTextureRegistry> *)regi
2121

2222
- (void)registerTexture:(NSObject<FlutterTexture> *)texture
2323
completion:(void (^)(int64_t))completion {
24+
__weak typeof(self) weakSelf = self;
2425
FLTEnsureToRunOnMainQueue(^{
25-
completion([self.registry registerTexture:texture]);
26+
typeof(self) strongSelf = weakSelf;
27+
if (!strongSelf) return;
28+
completion([strongSelf.registry registerTexture:texture]);
2629
});
2730
}
2831

2932
- (void)textureFrameAvailable:(int64_t)textureId {
33+
__weak typeof(self) weakSelf = self;
3034
FLTEnsureToRunOnMainQueue(^{
31-
[self.registry textureFrameAvailable:textureId];
35+
[weakSelf.registry textureFrameAvailable:textureId];
3236
});
3337
}
3438

3539
- (void)unregisterTexture:(int64_t)textureId {
40+
__weak typeof(self) weakSelf = self;
3641
FLTEnsureToRunOnMainQueue(^{
37-
[self.registry unregisterTexture:textureId];
42+
[weakSelf.registry unregisterTexture:textureId];
3843
});
3944
}
4045

0 commit comments

Comments
 (0)
0