Visitar URL original
ios: Return NO from run* calls on a running/terminated engine · flutter/flutter@52aefa1 · GitHub
Skip to content

Commit 52aefa1

Browse files
cbrackencbracken
committed
ios: Return NO from run* calls on a running/terminated engine
We explicitly document in FlutterEngine's `run*` methods that calling run more than once is will return without doing anything. In `[FlutterEngine destroyContext]` we specifically document that the engine is in an unusable state, that the engine will be in an unusable state until it is deallocated, and that accessing properties or sending messages to it will result in undefined behavior or runtime errors. By design, all calls to `run*` methods after the first invocation returned `NO`. This was unintentionally changed in flutter-team-archive/engine#6774 such that calling `run` on an already-running engine returns `YES` instead of `false` as it was prior to that patch. That's incorrect behaviour since it suggests that a new engine was spun up. A particularly obvious incorrect behaviour that this triggered was returning `YES` in the second invocation in a case like this: ```objc [engine runWithEntryPoint:@"main" ...]; [engine runWithEntryPoint:@"notMain" ...]; ``` This restores the original behaviour and prevents spinning up a new shell on a destroyed engine. We only ever documented that calling `run` a second time was unsupported and would return immediately with no effect, but never documented the return value on a second invocation, this updates that to document (and test) the intended behaviour. This also makes destroyContext idempotent such that calling it a second time is a no-op. This can happen if destroyContext is called directly as well as via e.g. `appOrSceneWillTerminate` or on view controller deallocation. This also adds null guards to unregisterTexture: and textureFrameAvailable: which would previously have triggered a null dereference. Related: flutter-team-archive/engine#6774 Related: flutter-team-archive/engine#7610 Issue: #185608 Issue: #115157 (b/256086107)
1 parent a388814 commit 52aefa1

3 files changed

Lines changed: 126 additions & 32 deletions

File tree

‎engine/src/flutter/shell/platform/darwin/ios/framework/Headers/FlutterEngine.h‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -174,21 +174,22 @@ FLUTTER_DARWIN_EXPORT
174174
* contains `main()`), using `main()` as the entrypoint (the default for Flutter projects),
175175
* and using "/" (the default route) as the initial route.
176176
*
177-
* The first call to this method will create a new Isolate. Subsequent calls will return
178-
* immediately and have no effect.
177+
* The first call to this method creates a new Isolate and returns YES. Subsequent calls return
178+
* NO immediately and have no effect.
179179
*
180180
* This method must be called on the main thread. See the `FlutterEngine` class documentation.
181181
*
182-
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
182+
* @return YES if this call started the engine; NO if it was already invoked, has been destroyed,
183+
* or could not be started.
183184
*/
184185
- (BOOL)run NS_SWIFT_UI_ACTOR;
185186

186187
/**
187188
* Runs a Dart program on an Isolate from the main Dart library (i.e. the library that
188189
* contains `main()`), using "/" (the default route) as the initial route.
189190
*
190-
* The first call to this method will create a new Isolate. Subsequent calls will return
191-
* immediately and have no effect.
191+
* The first call to this method creates a new Isolate and returns YES. Subsequent calls return
192+
* NO immediately and have no effect.
192193
*
193194
* This method must be called on the main thread. See the `FlutterEngine` class documentation.
194195
*
@@ -197,16 +198,17 @@ FLUTTER_DARWIN_EXPORT
197198
* nil) it will default to `main()`. If it is not the app's main() function, that function must
198199
* be decorated with `@pragma(vm:entry-point)` to ensure the method is not tree-shaken by the Dart
199200
* compiler.
200-
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
201+
* @return YES if this call started the engine; NO if it was already invoked, has been destroyed,
202+
* or could not be started.
201203
*/
202204
- (BOOL)runWithEntrypoint:(nullable NSString*)entrypoint NS_SWIFT_UI_ACTOR;
203205

204206
/**
205207
* Runs a Dart program on an Isolate from the main Dart library (i.e. the library that
206208
* contains `main()`).
207209
*
208-
* The first call to this method will create a new Isolate. Subsequent calls will return
209-
* immediately and have no effect.
210+
* The first call to this method creates a new Isolate and returns YES. Subsequent calls return
211+
* NO immediately and have no effect.
210212
*
211213
* This method must be called on the main thread. See the `FlutterEngine` class documentation.
212214
*
@@ -217,7 +219,8 @@ FLUTTER_DARWIN_EXPORT
217219
* compiler.
218220
* @param initialRoute The name of the initial Flutter `Navigator` `Route` to load. If this is
219221
* FlutterDefaultInitialRoute (or nil), it will default to the "/" route.
220-
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
222+
* @return YES if this call started the engine; NO if it was already invoked, has been destroyed,
223+
* or could not be started.
221224
*/
222225
- (BOOL)runWithEntrypoint:(nullable NSString*)entrypoint
223226
initialRoute:(nullable NSString*)initialRoute NS_SWIFT_UI_ACTOR;
@@ -226,8 +229,8 @@ FLUTTER_DARWIN_EXPORT
226229
* Runs a Dart program on an Isolate using the specified entrypoint and Dart library,
227230
* which may not be the same as the library containing the Dart program's `main()` function.
228231
*
229-
* The first call to this method will create a new Isolate. Subsequent calls will return
230-
* immediately and have no effect.
232+
* The first call to this method creates a new Isolate and returns YES. Subsequent calls return
233+
* NO immediately and have no effect.
231234
*
232235
* This method must be called on the main thread. See the `FlutterEngine` class documentation.
233236
*
@@ -238,7 +241,8 @@ FLUTTER_DARWIN_EXPORT
238241
* @param uri The URI of the Dart library which contains the entrypoint method
239242
* (example "package:foo_package/main.dart"). If nil, this will default to
240243
* the same library as the `main()` function in the Dart program.
241-
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
244+
* @return YES if this call started the engine; NO if it was already invoked, has been destroyed,
245+
* or could not be started.
242246
*/
243247
- (BOOL)runWithEntrypoint:(nullable NSString*)entrypoint
244248
libraryURI:(nullable NSString*)uri NS_SWIFT_UI_ACTOR;
@@ -247,8 +251,8 @@ FLUTTER_DARWIN_EXPORT
247251
* Runs a Dart program on an Isolate using the specified entrypoint and Dart library,
248252
* which may not be the same as the library containing the Dart program's `main()` function.
249253
*
250-
* The first call to this method will create a new Isolate. Subsequent calls will return
251-
* immediately and have no effect.
254+
* The first call to this method creates a new Isolate and returns YES. Subsequent calls return
255+
* NO immediately and have no effect.
252256
*
253257
* This method must be called on the main thread. See the `FlutterEngine` class documentation.
254258
*
@@ -261,7 +265,8 @@ FLUTTER_DARWIN_EXPORT
261265
* default to the same library as the `main()` function in the Dart program.
262266
* @param initialRoute The name of the initial Flutter `Navigator` `Route` to load. If this is
263267
* FlutterDefaultInitialRoute (or nil), it will default to the "/" route.
264-
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
268+
* @return YES if this call started the engine; NO if it was already invoked, has been destroyed,
269+
* or could not be started.
265270
*/
266271
- (BOOL)runWithEntrypoint:(nullable NSString*)entrypoint
267272
libraryURI:(nullable NSString*)libraryURI
@@ -271,8 +276,8 @@ FLUTTER_DARWIN_EXPORT
271276
* Runs a Dart program on an Isolate using the specified entrypoint and Dart library,
272277
* which may not be the same as the library containing the Dart program's `main()` function.
273278
*
274-
* The first call to this method will create a new Isolate. Subsequent calls will return
275-
* immediately and have no effect.
279+
* The first call to this method creates a new Isolate and returns YES. Subsequent calls return
280+
* NO immediately and have no effect.
276281
*
277282
* This method must be called on the main thread. See the `FlutterEngine` class documentation.
278283
*
@@ -286,7 +291,8 @@ FLUTTER_DARWIN_EXPORT
286291
* @param initialRoute The name of the initial Flutter `Navigator` `Route` to load. If this is
287292
* FlutterDefaultInitialRoute (or nil), it will default to the "/" route.
288293
* @param entrypointArgs Arguments passed as a list of string to Dart's entrypoint function.
289-
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
294+
* @return YES if this call started the engine; NO if it was already invoked, has been destroyed,
295+
* or could not be started.
290296
*/
291297
- (BOOL)runWithEntrypoint:(nullable NSString*)entrypoint
292298
libraryURI:(nullable NSString*)libraryURI
@@ -300,6 +306,8 @@ FLUTTER_DARWIN_EXPORT
300306
* After sending this message, the object will be in an unusable state until it is deallocated.
301307
* Accessing properties or sending messages to it will result in undefined behavior or runtime
302308
* errors.
309+
*
310+
* The engine cannot be run again: `-run` and the `-runWithEntrypoint:` variants return `NO`.
303311
*/
304312
- (void)destroyContext;
305313

‎engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ @interface FlutterEngine () <FlutterIndirectScribbleDelegate,
130130

131131
@property(nonatomic, readwrite, copy) NSString* isolateId;
132132
@property(nonatomic, copy) NSString* initialRoute;
133+
134+
// Whether the engine has been destroyed. A destroyed engine cannot be run again.
135+
@property(nonatomic, assign, getter=isDestroyed) BOOL destroyed;
133136
@property(nonatomic, strong) id<NSObject> flutterViewControllerWillDeallocObserver;
134137
@property(nonatomic, strong) FlutterDartVMServicePublisher* publisher;
135138
@property(nonatomic, strong) FlutterConnectionCollection* connections;
@@ -608,6 +611,10 @@ - (void)notifyViewControllerDeallocated {
608611
}
609612

610613
- (void)destroyContext {
614+
if (self.destroyed) {
615+
return;
616+
}
617+
self.destroyed = YES;
611618
[self resetChannels];
612619
self.isolateId = nil;
613620
_shell.reset();
@@ -923,6 +930,12 @@ - (BOOL)createShell:(NSString*)entrypoint
923930
"its platform and UI thread, both of which must be the main thread. To start an engine "
924931
"from a background queue, dispatch to the main queue first.";
925932

933+
if (self.destroyed) {
934+
[FlutterLogger
935+
logWarning:@"This FlutterEngine was destroyed by destroyContext and cannot be run again."];
936+
return NO;
937+
}
938+
926939
if (_shell != nullptr) {
927940
[FlutterLogger logWarning:@"This FlutterEngine was already invoked."];
928941
return NO;
@@ -1056,11 +1069,11 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint
10561069
libraryURI:(NSString*)libraryURI
10571070
initialRoute:(NSString*)initialRoute
10581071
entrypointArgs:(NSArray<NSString*>*)entrypointArgs {
1059-
if ([self createShell:entrypoint libraryURI:libraryURI initialRoute:initialRoute]) {
1060-
[self launchEngine:entrypoint libraryURI:libraryURI entrypointArgs:entrypointArgs];
1072+
if (![self createShell:entrypoint libraryURI:libraryURI initialRoute:initialRoute]) {
1073+
return NO;
10611074
}
1062-
1063-
return _shell != nullptr;
1075+
[self launchEngine:entrypoint libraryURI:libraryURI entrypointArgs:entrypointArgs];
1076+
return YES;
10641077
}
10651078

10661079
- (void)notifyLowMemory {
@@ -1423,11 +1436,19 @@ - (int64_t)registerTexture:(NSObject<FlutterTexture>*)texture {
14231436
}
14241437

14251438
- (void)unregisterTexture:(int64_t)textureId {
1426-
_shell->GetPlatformView()->UnregisterTexture(textureId);
1439+
flutter::PlatformViewIOS* platform_view = self.platformView;
1440+
if (!platform_view) {
1441+
return;
1442+
}
1443+
platform_view->UnregisterTexture(textureId);
14271444
}
14281445

14291446
- (void)textureFrameAvailable:(int64_t)textureId {
1430-
_shell->GetPlatformView()->MarkTextureFrameAvailable(textureId);
1447+
flutter::PlatformViewIOS* platform_view = self.platformView;
1448+
if (!platform_view) {
1449+
return;
1450+
}
1451+
platform_view->MarkTextureFrameAvailable(textureId);
14311452
}
14321453

14331454
- (NSString*)lookupKeyForAsset:(NSString*)asset {
@@ -1744,14 +1765,14 @@ - (void)addMethodCallDelegate:(NSObject<FlutterPlugin>*)delegate
17441765
static BOOL FLTFlutterPluginRespondsToLegacyAppLifecycleSelectors(
17451766
NSObject<FlutterPlugin>* delegate) {
17461767
SEL selectors[] = {
1747-
@selector(applicationDidBecomeActive:),
1748-
@selector(applicationWillResignActive:),
1749-
@selector(applicationWillEnterForeground:),
1750-
@selector(applicationDidEnterBackground:),
1751-
@selector(application:continueUserActivity:restorationHandler:),
1752-
@selector(application:performActionForShortcutItem:completionHandler:),
1753-
@selector(application:openURL:options:),
1754-
@selector(application:performFetchWithCompletionHandler:),
1768+
@selector(applicationDidBecomeActive:),
1769+
@selector(applicationWillResignActive:),
1770+
@selector(applicationWillEnterForeground:),
1771+
@selector(applicationDidEnterBackground:),
1772+
@selector(application:continueUserActivity:restorationHandler:),
1773+
@selector(application:performActionForShortcutItem:completionHandler:),
1774+
@selector(application:openURL:options:),
1775+
@selector(application:performFetchWithCompletionHandler:),
17551776
};
17561777
for (SEL sel : selectors) {
17571778
if ([delegate respondsToSelector:sel]) {

‎engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@
4545
};
4646
} // namespace
4747

48+
/// A `FlutterTexture` that vends no frames. Used to obtain a registered texture id.
49+
@interface TestFlutterTexture : NSObject <FlutterTexture>
50+
@end
51+
52+
@implementation TestFlutterTexture
53+
- (CVPixelBufferRef _Nullable)copyPixelBuffer {
54+
return nullptr;
55+
}
56+
@end
57+
4858
@protocol TestFlutterPluginWithSceneEvents <NSObject, FlutterPlugin, FlutterSceneLifeCycleDelegate>
4959
@end
5060

@@ -133,6 +143,61 @@ - (void)testTaskRunnerPropertyStability {
133143
XCTAssertEqual(engine.rasterTaskRunner, engine.rasterTaskRunner);
134144
}
135145

146+
- (void)testRunOnRunningEngineIsRefused {
147+
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:nil];
148+
XCTAssertTrue([engine run]);
149+
FlutterFMLTaskRunner* platformTaskRunner = engine.platformTaskRunner;
150+
151+
// Verify a second call reports that it did not restart the engine.
152+
XCTAssertFalse([engine run]);
153+
XCTAssertEqual(engine.platformTaskRunner, platformTaskRunner);
154+
}
155+
156+
- (void)testRunWithDifferentEntrypointOnRunningEngineIsRefused {
157+
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:nil];
158+
XCTAssertTrue([engine run]);
159+
160+
// Verify a second call reports that it did not restart the engine with a new entrypoint.
161+
XCTAssertFalse([engine runWithEntrypoint:@"someOtherEntrypoint"]);
162+
}
163+
164+
- (void)testRunAfterDestroyContextIsRefused {
165+
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:nil];
166+
XCTAssertTrue([engine run]);
167+
[engine destroyContext];
168+
169+
// A destroyed engine releases the shell, so the `_shell != nullptr` guard against double
170+
// invocation no longer catches a second run. Verify the destroyed state is refused on its own.
171+
XCTAssertFalse([engine run]);
172+
XCTAssertNil(engine.platformTaskRunner);
173+
XCTAssertNil(engine.uiTaskRunner);
174+
XCTAssertNil(engine.rasterTaskRunner);
175+
}
176+
177+
- (void)testDestroyContextIsIdempotent {
178+
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:nil];
179+
XCTAssertTrue([engine run]);
180+
181+
// Both `appOrSceneWillTerminate` and view controller deallocation can destroy the same engine.
182+
// Verify a second call is a no-op and not a double teardown.
183+
[engine destroyContext];
184+
[engine destroyContext];
185+
XCTAssertNil(engine.platformTaskRunner);
186+
}
187+
188+
- (void)testTextureCallbacksAfterDestroyContextAreIgnored {
189+
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:nil];
190+
XCTAssertTrue([engine run]);
191+
NSObject<FlutterTextureRegistry>* textures = [[engine registrarForPlugin:@"test"] textures];
192+
int64_t textureId = [textures registerTexture:[[TestFlutterTexture alloc] init]];
193+
[engine destroyContext];
194+
195+
// Texture producers outlive the shell. Verify a frame callback arriving after teardown or during
196+
// app termination don't crash.
197+
[textures textureFrameAvailable:textureId];
198+
[textures unregisterTexture:textureId];
199+
}
200+
136201
- (void)testInfoPlist {
137202
// Check the embedded Flutter.framework Info.plist, not the linked dylib.
138203
NSURL* flutterFrameworkURL =

0 commit comments

Comments
 (0)