[iOS] Fabric: Fixes assert failure when surface stop before we start surface

[zhongwuzw]

Summary:

Fixes #48149. Actually this issue is not caused by React 19. The underlying problem arises because we retain the surface in RCTHost at the time of its creation. Even though we call stop, there is a return check that prevents execution if the status is not running. For reference, you can view the relevant code here: RCTFabricSurface.mm.

To resolve this issue, we can implement a weak reference to the surface.

bt:

(lldb) bt
* thread #13, queue = 'com.apple.root.user-interactive-qos', stop reason = signal SIGABRT
    frame #0: 0x0000000105699008 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x00000001045df408 libsystem_pthread.dylib`pthread_kill + 256
    frame #2: 0x000000018016c4ec libsystem_c.dylib`abort + 104
    frame #3: 0x000000018016b934 libsystem_c.dylib`__assert_rtn + 268
    frame #4: 0x000000010651f4b4 React_Fabric`facebook::react::SurfaceHandler::setUIManager(this=0x0000000108108620, uiManager=0x0000000000000000) const at SurfaceHandler.cpp:317:3
    frame #5: 0x00000001064c98f4 React_Fabric`facebook::react::Scheduler::unregisterSurface(this=0x0000600003500370, surfaceHandler=0x0000000108108620) const at Scheduler.cpp:252:18
    frame #6: 0x0000000104c3e8a8 RCTFabric`-[RCTScheduler unregisterSurface:](self=0x000060000212a940, _cmd="unregisterSurface:", surfaceHandler=0x0000000108108620) at RCTScheduler.mm:163:15
    frame #7: 0x0000000104c61fc4 RCTFabric`-[RCTSurfacePresenter unregisterSurface:](self=0x00000001081080d0, _cmd="unregisterSurface:", surface=0x0000000108108610) at RCTSurfacePresenter.mm:126:5
  * frame #8: 0x0000000104bc30a0 RCTFabric`-[RCTFabricSurface dealloc](self=0x0000000108108610, _cmd="dealloc") at RCTFabricSurface.mm:87:3
    frame #9: 0x0000000104b9ae44 RCTFabric`__destroy_helper_block_ea8_32s((null)=0x0000600000cb5a70) at RCTBoxShadow.mm:0
    frame #10: 0x00000001800f6edc libsystem_blocks.dylib`_call_dispose_helpers_excp + 44
    frame #11: 0x00000001800f7d24 libsystem_blocks.dylib`_Block_release + 300
    frame #12: 0x000000010760a7b8 libdispatch.dylib`_dispatch_client_callout + 16
    frame #13: 0x000000010761e608 libdispatch.dylib`_dispatch_root_queue_drain + 936
    frame #14: 0x000000010761ef7c libdispatch.dylib`_dispatch_worker_thread2 + 256
    frame #15: 0x00000001045dbb38 libsystem_pthread.dylib`_pthread_wqthread + 224

Changelog:

[IOS] [FIXED] - Fabric: Fixes assert failure when surface stop before we start surface

Test Plan:

Please see demo in #48149. Or open RNTester and apply the patch like below:

diff --git a/packages/rn-tester/RNTester/AppDelegate.mm b/packages/rn-tester/RNTester/AppDelegate.mm
index 64c5d4122e7..cf015458619 100644
--- a/packages/rn-tester/RNTester/AppDelegate.mm
+++ b/packages/rn-tester/RNTester/AppDelegate.mm
@@ -50,7 +50,10 @@ static NSString *kBundlePath = @"js/RNTesterApp.ios";
 
   [[UNUserNotificationCenter currentNotificationCenter] setDelegate:self];
 
-  return [super application:application didFinishLaunchingWithOptions:launchOptions];
+  [super application:application didFinishLaunchingWithOptions:launchOptions];
+  self.window.rootViewController = [UIViewController new];
+  [self.window makeKeyAndVisible];
+  return YES;
 }
 
 - (void)applicationDidEnterBackground:(UIApplication *)application

[javache]

I'm not actually sure why this needs go through the JS thread, since start already dispatches async internally anyway and is threadsafe. It seems this was added in #45486 ?

Can we make this just [surface start] and remove the weak ref? It not, can you add a comment on why this requires synchronization on the JS thread?

[zhongwuzw]

@javache Hi, I added the comment, [surface start] needs to wait for the main bundle to be loaded, so we dispatched it on the buffered runtime executor.

[javache]

I'm a bit surprised by that. ReactInstance should be able to deal with the queuing required to delay surface start until the bundle is ready.

[zhongwuzw]

The surface start seems just schedule to the js thread :

react-native/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp

Lines 238 to 242 in f4a17fb

	runtimeExecutor_([=](jsi::Runtime& runtime) {
	SystraceSection s("UIManager::startSurface::onRuntime");
	SurfaceRegistryBinding::startSurface(
	runtime, surfaceId, moduleName, props, displayMode);
	});