Skip to content

Commit

Permalink
Fabric: Fixes assert failure when surface stop before we start surface (
Browse files Browse the repository at this point in the history
#48213)

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](https://github.com/facebook/react-native/blob/7d771de8a79b05e8dfed91e07de30d9f72d3c1c3/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm#L118).

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

Pull Request resolved: #48213

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

```

Reviewed By: javache

Differential Revision: D67335792

Pulled By: cipolleschi

fbshipit-source-id: e93aaaa60b3d204d7ed2cda6758b3b1d9dfcbc88
  • Loading branch information
zhongwuzw authored and facebook-github-bot committed Dec 24, 2024
1 parent adaceba commit 6d2b616
Showing 1 changed file with 3 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,9 @@ - (RCTFabricSurface *)createSurfaceWithModuleName:(NSString *)moduleName
surface.surfaceHandler.setDisplayMode(displayMode);
[self _attachSurface:surface];

[_instance callFunctionOnBufferedRuntimeExecutor:[surface](facebook::jsi::Runtime &_) { [surface start]; }];
__weak RCTFabricSurface *weakSurface = surface;
// Use the BufferedRuntimeExecutor to start the surface after the main JS bundle was fully executed.
[_instance callFunctionOnBufferedRuntimeExecutor:[weakSurface](facebook::jsi::Runtime &_) { [weakSurface start]; }];
return surface;
}

Expand Down

0 comments on commit 6d2b616

Please sign in to comment.