[iOS] Destroy the engine prior to application termination.

[zhongwuzw]

Try fixes flutter/flutter#90783.
Based on the full crash log, Flutter tries to use std::mutex when App is terminating. we can fix it by leaking global static mutex. React Native and ComponentKit also faced this issue, we can refer to them.

cc. @chinmaygarde

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[chinmaygarde]

I don't think this will fix crashing. It will just move the crash to some other spot. In fact, in the absence of graceful termination, I don't think there is a way to avoid crashes during termination.

Apple's Technical Q&A QA1561 explicitly states "Do not call the exit function. Applications calling exit will appear to the user to have crashed, rather than performing a graceful termination and animating back to the Home screen."

So, is the crash happening because someone called exit or because the engine is not reacting to graceful termination? If it's the former, the fix is to stop whoever is calling exit from doing so. If it's the latter, we need to figure out how that happens and gracefully terminate the engine.

[zhongwuzw]

I don't think this will fix crashing. It will just move the crash to some other spot. In fact, in the absence of graceful termination, I don't think there is a way to avoid crashes during termination.

Apple's Technical Q&A QA1561 explicitly states "Do not call the exit function. Applications calling exit will appear to the user to have crashed, rather than performing a graceful termination and animating back to the Home screen."

So, is the crash happening because someone called exit or because the engine is not reacting to graceful termination? If it's the former, the fix is to stop whoever is calling exit from doing so. If it's the latter, we need to figure out how that happens and gracefully terminate the engine.

@chinmaygarde You're right, we may need to change all global mutex to heap allocated or handle the termination gracefully. From the backtrace of crash log, I think the crash happening because engine is not reacting to graceful termination, case like user swipe up from the bottom into app switcher, then kill the App directly, seems we didn't handle this case, I think we can destroy the engine when we check app will terminate, the code like below .

diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm
index cc5ac62d3..0a349b25a 100644
--- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm
+++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm
@@ -275,6 +275,11 @@ typedef enum UIAccessibilityContrast : NSInteger {
              selector:@selector(applicationWillResignActive:)
                  name:UIApplicationWillResignActiveNotification
                object:nil];
+  
+  [center addObserver:self
+             selector:@selector(applicationWillTerminate:)
+                 name:UIApplicationWillTerminateNotification
+               object:nil];
 
   [center addObserver:self
              selector:@selector(applicationDidEnterBackground:)
@@ -790,6 +795,10 @@ static void sendFakeTouchEvent(FlutterEngine* engine,
   [self goToApplicationLifecycle:@"AppLifecycleState.inactive"];
 }
 
+- (void)applicationWillTerminate:(NSNotification*)notification {
+  [self goToApplicationLifecycle:@"AppLifecycleState.detached"];
+  [self.engine destroyContext];
+}
+
 - (void)applicationDidEnterBackground:(NSNotification*)notification {
   TRACE_EVENT0("flutter", "applicationDidEnterBackground");
   [self surfaceUpdated:NO];

[chinmaygarde]

I think the crash happening because engine is not reacting to graceful termination, case like user swipe up from the bottom into app switcher, then kill the App directly, seems we didn't handle this case,

Right, this seems to the most likely cause of the error. This would also explain why it's hard to observe or reproduce. Changing all global mutexes to be heap allocated and leaked will also not help because those mutexes guard process global information in the VM. So I think graceful termination is the only way to go. You patch to do so makes a lot of sense to me. Have you been able to reproduce the crash with your error reporting solution and swiping the app up to terminate it? If so, we can pretty confidently say we have the reproducible case.

[zhongwuzw]

Have you been able to reproduce the crash with your error reporting solution and swiping the app up to terminate it? If so, we can pretty confidently say we have the reproducible case.

@chinmaygarde Sorry, I didn't reproduce natively, but I think as long as we ensure terminate the engine before App called static global variables destructor, we can avoid the crash. UIApplicationWillTerminateNotification is called before __cxa_finalize_ranges,so we can do engine's termination when observed UIApplicationWillTerminateNotification.

Thread 0 name:
Thread 0:
0   BBRtc                         	0x0000000104bad558 0x104afc000 + 726360
1   libsystem_c.dylib             	0x00000001a831c0a8 __cxa_finalize_ranges + 408 (atexit.c:284)
2   libsystem_c.dylib             	0x00000001a831c400 exit + 28 (exit.c:81)
3   UIKitCore                     	0x00000001a18ceae4 -[UIApplication _terminateWithStatus:] + 504 (UIApplication.m:6510)
4   UIKitCore                     	0x00000001a0f1d39c -[_UISceneLifecycleMultiplexer _evalTransitionToSettings:fromSettings:forceExit:withTransitionStore:] + 128 (_UISceneLifecycleMultiplexer.m:831)
5   UIKitCore                     	0x00000001a0f1cfcc -[_UISceneLifecycleMultiplexer forceExitWithTransitionContext:scene:] + 220 (_UISceneLifecycleMultiplexer.m:466)
6   UIKitCore                     	0x00000001a18c454c -[UIApplication workspaceShouldExit:withTransitionContext:] + 212 (UIApplication.m:3583)
7   FrontBoardServices            	0x00000001aec75780 -[FBSUIApplicationWorkspaceShim workspaceShouldExit:withTransitionContext:] + 88 (FBSUIApplicationWorkspace.m:144)
8   FrontBoardServices            	0x00000001aeca4390 __63-[FBSWorkspaceScenesClient willTerminateWithTransitionContext:]_block_invoke_2 + 80 (FBSWorkspaceScenesClient.m:312)
9   FrontBoardServices            	0x00000001aec884a0 -[FBSWorkspace _calloutQueue_executeCalloutFromSource:withBlock:] + 240 (FBSWorkspace.m:355)
10  FrontBoardServices            	0x00000001aeca4328 __63-[FBSWorkspaceScenesClient willTerminateWithTransitionContext:]_block_invoke + 132 (FBSWorkspaceScenesClient.m:309)
11  libdispatch.dylib             	0x000000019eb01db0 _dispatch_client_callout + 20 (object.m:559)
12  libdispatch.dylib             	0x000000019eb05738 _dispatch_block_invoke_direct + 268 (queue.c:468)
13  FrontBoardServices            	0x00000001aeccd250 __FBSSERIALQUEUE_IS_CALLING_OUT_TO_A_BLOCK__ + 48 (FBSSerialQueue.m:184)
14  FrontBoardServices            	0x00000001aecccee0 -[FBSSerialQueue _targetQueue_performNextIfPossible] + 448 (FBSSerialQueue.m:227)
15  FrontBoardServices            	0x00000001aeccd434 -[FBSSerialQueue _performNextFromRunLoopSource] + 32 (FBSSerialQueue.m:258)
16  CoreFoundation                	0x000000019ee8976c __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 28 (CFRunLoop.c:1967)
17  CoreFoundation                	0x000000019ee89668 __CFRunLoopDoSource0 + 208 (CFRunLoop.c:2011)
18  CoreFoundation                	0x000000019ee88960 __CFRunLoopDoSources0 + 268 (CFRunLoop.c:2048)
19  CoreFoundation                	0x000000019ee82a8c __CFRunLoopRun + 824 (CFRunLoop.c:2925)
20  CoreFoundation                	0x000000019ee8221c CFRunLoopRunSpecific + 600 (CFRunLoop.c:3242)
21  GraphicsServices              	0x00000001b6a4f784 GSEventRunModal + 164 (GSEvent.c:2259)
22  UIKitCore                     	0x00000001a18c2ee8 -[UIApplication _run] + 1072 (UIApplication.m:3253)
23  UIKitCore                     	0x00000001a18c875c UIApplicationMain + 168 (UIApplication.m:4707)
24  Runner                        	0x0000000102140534 0x102138000 + 34100
25  libdyld.dylib                 	0x000000019eb426b0 start + 4

[zhongwuzw]

macOS desktop also has a similar crash, which also happened in App closing, C++ destruction stage.

[The-Redhat]

Hey @zhongwuzw @chinmaygarde do you currently have a rough eta when this fix will land?

[zhongwuzw]

Hey @zhongwuzw @chinmaygarde do you currently have a rough eta when this fix will land?

Sorry, no eta currently, still under review.

[chinmaygarde]

I edited commit message to indicate what is actually going on in this patch. Othewise, LGTM.

[fluttergithubbot]

This pull request is not suitable for automatic merging in its current state.

• The status or check suite Mac Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.