Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit fcf65a6

Browse files
committed
iOS: Add null checks on shell dereference
`FlutterEngine` at the `_shell` unique_ptr ivar it owns have different lifetimes. `_shell` is initialised transitively from `runWithEntrypoint`, and reset in `[FlutterEngine destroyContext]`, which is called transitively from `[FlutterviewController dealloc]` via `[FlutterEngine notifyViewControllerDeallocated]`. As such, all uses of `_shell` should be checked either via an assertion, in cases we know the shell should be non-null, or via a runtime null check in cases where it's expected that it may be null. Specifically, this guards against a crash that can occur if we get a CoreAnimation transaction commit callback for an inflight frame just as we're shutting down the app (or removing the FlutterView in an add-to-app scenario). Example stack trace: ``` 0 Flutter 0x11b28 -[FlutterEngine platformView] + 53 (weak_ptr.h:53) 1 Flutter 0x11994 -[FlutterEngine updateViewportMetrics:] + 186 (ref_ptr.h:186) 2 Flutter 0x1f854 -[FlutterViewController updateViewportMetricsIfNeeded] + 427 (vector:427) 3 Flutter 0x1f9b8 -[FlutterViewController viewDidLayoutSubviews] + 1411 (FlutterViewController.mm:1411) 4 UIKitCore 0x8c864 -[UIView(CALayerDelegate) layoutSublayersOfLayer:] + 2376 5 QuartzCore 0x1fa0c CA::Layer::layout_if_needed(CA::Transaction*) + 516 6 QuartzCore 0x1ae84c CA::Context::commit_transaction(CA::Transaction*, double, double*) + 516 7 QuartzCore 0x2888 CA::Transaction::commit() + 648 ``` Issue: flutter/flutter#159639
1 parent 4d00124 commit fcf65a6

File tree

4 files changed

+41
-8
lines changed

4 files changed

+41
-8
lines changed

shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -320,27 +320,37 @@ - (void)dispatchPointerDataPacket:(std::unique_ptr<flutter::PointerDataPacket>)p
320320
}
321321

322322
- (fml::WeakPtr<flutter::PlatformView>)platformView {
323-
FML_DCHECK(_shell);
323+
if (!_shell) {
324+
return {};
325+
}
324326
return _shell->GetPlatformView();
325327
}
326328

327329
- (flutter::PlatformViewIOS*)iosPlatformView {
328-
FML_DCHECK(_shell);
330+
if (!_shell) {
331+
return nullptr;
332+
}
329333
return static_cast<flutter::PlatformViewIOS*>(_shell->GetPlatformView().get());
330334
}
331335

332336
- (fml::RefPtr<fml::TaskRunner>)platformTaskRunner {
333-
FML_DCHECK(_shell);
337+
if (!_shell) {
338+
return {};
339+
}
334340
return _shell->GetTaskRunners().GetPlatformTaskRunner();
335341
}
336342

337343
- (fml::RefPtr<fml::TaskRunner>)uiTaskRunner {
338-
FML_DCHECK(_shell);
344+
if (!_shell) {
345+
return {};
346+
}
339347
return _shell->GetTaskRunners().GetUITaskRunner();
340348
}
341349

342350
- (fml::RefPtr<fml::TaskRunner>)rasterTaskRunner {
343-
FML_DCHECK(_shell);
351+
if (!_shell) {
352+
return {};
353+
}
344354
return _shell->GetTaskRunners().GetRasterTaskRunner();
345355
}
346356

@@ -392,6 +402,9 @@ - (void)sendKeyEvent:(const FlutterKeyEvent&)event
392402
}
393403

394404
- (void)ensureSemanticsEnabled {
405+
if (!self.iosPlatformView) {
406+
return;
407+
}
395408
self.iosPlatformView->SetSemanticsEnabled(true);
396409
}
397410

@@ -419,6 +432,7 @@ - (void)setViewController:(FlutterViewController*)viewController {
419432
}
420433

421434
- (void)attachView {
435+
FML_DCHECK(self.iosPlatformView);
422436
self.iosPlatformView->attachView();
423437
}
424438

@@ -1224,6 +1238,7 @@ - (void)cleanUpConnection:(FlutterBinaryMessengerConnection)connection {
12241238
#pragma mark - FlutterTextureRegistry
12251239

12261240
- (int64_t)registerTexture:(NSObject<FlutterTexture>*)texture {
1241+
FML_DCHECK(self.iosPlatformView);
12271242
int64_t textureId = self.nextTextureId++;
12281243
self.iosPlatformView->RegisterExternalTexture(textureId, texture);
12291244
return textureId;

shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,19 @@ - (void)testCreate {
6969
XCTAssertNotNil(engine);
7070
}
7171

72+
- (void)testShellGetters {
73+
FlutterDartProject* project = [[FlutterDartProject alloc] init];
74+
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project];
75+
XCTAssertNotNil(engine);
76+
77+
// Ensure getters don't deref _shell when it's null, and instead return nullptr.
78+
XCTAssertEqual(engine.platformView.get(), nullptr);
79+
XCTAssertEqual(engine.iosPlatformView, nullptr);
80+
XCTAssertEqual(engine.platformTaskRunner.get(), nullptr);
81+
XCTAssertEqual(engine.uiTaskRunner.get(), nullptr);
82+
XCTAssertEqual(engine.rasterTaskRunner.get(), nullptr);
83+
}
84+
7285
- (void)testInfoPlist {
7386
// Check the embedded Flutter.framework Info.plist, not the linked dylib.
7487
NSURL* flutterFrameworkURL =

shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,8 @@ - (void)installFirstFrameCallback {
653653
weakPlatformView->SetNextFrameCallback([weakSelf,
654654
platformTaskRunner = self.engine.platformTaskRunner,
655655
rasterTaskRunner = self.engine.rasterTaskRunner]() {
656+
FML_DCHECK(platformTaskRunner);
657+
FML_DCHECK(rasterTaskRunner);
656658
FML_DCHECK(rasterTaskRunner->RunsTasksOnCurrentThread());
657659
// Get callback on raster thread and jump back to platform thread.
658660
platformTaskRunner->PostTask([weakSelf]() { [weakSelf onFirstFrameRendered]; });
@@ -1822,7 +1824,7 @@ - (void)setUpKeyboardAnimationVsyncClient:
18221824
});
18231825
};
18241826

1825-
_keyboardAnimationVSyncClient = [[VSyncClient alloc] initWithTaskRunner:[self.engine uiTaskRunner]
1827+
_keyboardAnimationVSyncClient = [[VSyncClient alloc] initWithTaskRunner:self.engine.uiTaskRunner
18261828
callback:uiCallback];
18271829
_keyboardAnimationVSyncClient.allowPauseAfterVsync = NO;
18281830
[_keyboardAnimationVSyncClient await];
@@ -2105,6 +2107,9 @@ - (void)onAccessibilityStatusChanged:(NSNotification*)notification {
21052107
return;
21062108
}
21072109
fml::WeakPtr<flutter::PlatformView> platformView = self.engine.platformView;
2110+
if (!platformView) {
2111+
return;
2112+
}
21082113
int32_t flags = self.accessibilityFlags;
21092114
#if TARGET_OS_SIMULATOR
21102115
// There doesn't appear to be any way to determine whether the accessibility

shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ @implementation VSyncClient {
7070

7171
- (instancetype)initWithTaskRunner:(fml::RefPtr<fml::TaskRunner>)task_runner
7272
callback:(flutter::VsyncWaiter::Callback)callback {
73-
self = [super init];
73+
FML_DCHECK(task_runner);
7474

75-
if (self) {
75+
if (self = [super init]) {
7676
_refreshRate = DisplayLinkManager.displayRefreshRate;
7777
_allowPauseAfterVsync = YES;
7878
_callback = std::move(callback);

0 commit comments

Comments
 (0)