Skip to content

Commit

Permalink
Fixes main thread stuck when reload in bridgeless mode (#45486)
Browse files Browse the repository at this point in the history 
Summary:
In fabric bridgeless mode, when we reload, main thread may block because of dead lock. the backtrace example as below:

```
(lldb) bt
* thread #1, stop reason = signal SIGSTOP
  * frame #0: 0x000000010a5c76f2 libsystem_kernel.dylib`__psynch_mutexwait + 10
    frame #1: 0x00000001099e0a70 libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_wait + 78
    frame #2: 0x00000001099de82b libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_slow + 217
    frame #3: 0x00007ff80030c6b9 libc++.1.dylib`std::__1::mutex::lock() + 9
    frame #4: 0x0000000106968b13 RNTester`std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:ue170006](this=0x00007ff7b95e2478, __m=0x000060000377c958) at lock_guard.h:35:10
    frame #5: 0x00000001069689ed RNTester`std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:ue170006](this=0x00007ff7b95e2478, __m=0x000060000377c958) at lock_guard.h:34:19
    frame #6: 0x00000001070691c1 RNTester`-[RCTInstance invalidate](self=0x000060000377c900, _cmd="invalidate") at RCTInstance.mm:146:31
    frame #7: 0x0000000107060fd2 RNTester`-[RCTHost didReceiveReloadCommand](self=0x0000600003d100f0, _cmd="didReceiveReloadCommand") at RCTHost.mm:317:3
    frame #8: 0x0000000106b005a5 RNTester`RCTTriggerReloadCommandListeners(reason=@"Global hotkey") at RCTReloadCommand.m:57:5
    frame #9: 0x0000000106b86da5 RNTester`__28-[RCTDevSettings initialize]_block_invoke.157(.block_descriptor=0x0000000107496170, params=0x00007ff84002f610) at RCTDevSettings.mm:201:11
    frame #10: 0x0000000106ae658e RNTester`__65-[RCTPackagerConnection reconnectingWebSocket:didReceiveMessage:]_block_invoke.68(.block_descriptor=0x0000600000c82df0) at RCTPackagerConnection.mm:293:9
    frame #11: 0x0000000109a4529d libdispatch.dylib`_dispatch_call_block_and_release + 12
    frame #12: 0x0000000109a4658f libdispatch.dylib`_dispatch_client_callout + 8
    frame #13: 0x0000000109a563ee libdispatch.dylib`_dispatch_main_queue_drain + 1362
    frame #14: 0x0000000109a55e8e libdispatch.dylib`_dispatch_main_queue_callback_4CF + 31
    frame #15: 0x00007ff800429af4 CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
    frame #16: 0x00007ff80042442f CoreFoundation`__CFRunLoopRun + 2463
    frame #17: 0x00007ff8004236ad CoreFoundation`CFRunLoopRunSpecific + 557
    frame #18: 0x00007ff8103da08f GraphicsServices`GSEventRunModal + 137
    frame #19: 0x00007ff805cc0ad1 UIKitCore`-[UIApplication _run] + 972
    frame #20: 0x00007ff805cc5551 UIKitCore`UIApplicationMain + 123
    frame #21: 0x00000001069205a0 RNTester`main(argc=1, argv=0x00007ff7b95e3b60) at main.m:15:12
    frame #22: 0x00000001099023e0 dyld_sim`start_sim + 10
    frame #23: 0x0000000116b92366 dyld`start + 1942

(lldb) bt
* thread #3
    frame #0: 0x000000010a5c6b86 libsystem_kernel.dylib`__ulock_wait + 10
    frame #1: 0x0000000109a46eb1 libdispatch.dylib`_dlock_wait + 46
    frame #2: 0x0000000109a46d08 libdispatch.dylib`_dispatch_thread_event_wait_slow + 40
    frame #3: 0x0000000109a5774a libdispatch.dylib`__DISPATCH_WAIT_FOR_QUEUE__ + 371
    frame #4: 0x0000000109a57161 libdispatch.dylib`_dispatch_sync_f_slow + 240
    frame #5: 0x0000000106b3b33b RNTester`RCTUnsafeExecuteOnMainQueueSync(block=0x0000000106f116c0) at RCTUtils.m:291:5
  * frame #6: 0x0000000106f115ad RNTester`-[RCTFabricSurface start](self=0x000000010af0df40, _cmd="start") at RCTFabricSurface.mm:102:3
    frame #7: 0x00000001070601ce RNTester`__108-[RCTHost initWithBundleURLProvider:hostDelegate:turboModuleManagerDelegate:jsEngineProvider:launchOptions:]_block_invoke_2(.block_descriptor=0x0000600000c75590) at RCTHost.mm:211:9
    frame #8: 0x000000010706cdc8 RNTester`-[RCTInstance _loadScriptFromSource:](self=0x000060000377c900, _cmd="_loadScriptFromSource:", source=0x0000600000cd57d0) at RCTInstance.mm:472:5
    frame #9: 0x000000010706ca81 RNTester`__29-[RCTInstance _loadJSBundle:]_block_invoke.120(.block_descriptor=0x0000600000c96d00, error=0x0000000000000000, source=0x0000600000cd57d0) at RCTInstance.mm:452:9
    frame #10: 0x0000000106ab1919 RNTester`invocation function for block in attemptAsynchronousLoadOfBundleAtURL(.block_descriptor=0x00006000017b0fc0, statusCode=200, headers=6 key/value pairs, data=0x00006000002a4760, error=0x0000000000000000, done=YES) block_pointer, void (NSError*, RCTSource*) block_pointer) at RCTJavaScriptLoader.mm:318:9
    frame #11: 0x0000000106ad92a6 RNTester`__80-[RCTMultipartDataTask URLSession:streamTask:didBecomeInputStream:outputStream:]_block_invoke(.block_descriptor=0x000070000035c7a0, headers=6 key/value pairs, content=0x00006000002a4760, done=YES) at RCTMultipartDataTask.m:121:9
    frame #12: 0x0000000106ad9b4f RNTester`-[RCTMultipartStreamReader emitChunk:headers:callback:done:](self=0x00006000002b4220, _cmd="emitChunk:headers:callback:done:", data=0x00006000002a4020, headers=6 key/value pairs, callback=0x0000000106ad9230, done=YES) at RCTMultipartStreamReader.m:57:5
    frame #13: 0x0000000106ada800 RNTester`-[RCTMultipartStreamReader readAllPartsWithCompletionCallback:progressCallback:](self=0x00006000002b4220, _cmd="readAllPartsWithCompletionCallback:progressCallback:", callback=0x0000000106ad9230, progressCallback=0x0000000106ab2a60) at RCTMultipartStreamReader.m:154:7
    frame #14: 0x0000000106ad9130 RNTester`-[RCTMultipartDataTask URLSession:streamTask:didBecomeInputStream:outputStream:](self=0x00006000017b0d40, _cmd="URLSession:streamTask:didBecomeInputStream:outputStream:", session=0x000000010e02a0a0, streamTask=0x000000010c83ba00, inputStream=0x000060000300d4d0, outputStream=0x000060000300c990) at RCTMultipartDataTask.m:119:20
    frame #15: 0x00007ff80479fdf9 CFNetwork`___lldb_unnamed_symbol2876 + 42
    frame #16: 0x0000000109a4529d libdispatch.dylib`_dispatch_call_block_and_release + 12
    frame #17: 0x0000000109a4658f libdispatch.dylib`_dispatch_client_callout + 8
    frame #18: 0x0000000109a4e4ba libdispatch.dylib`_dispatch_lane_serial_drain + 1127
    frame #19: 0x0000000109a4f255 libdispatch.dylib`_dispatch_lane_invoke + 441
    frame #20: 0x0000000109a5c356 libdispatch.dylib`_dispatch_root_queue_drain_deferred_wlh + 318
    frame #21: 0x0000000109a5b751 libdispatch.dylib`_dispatch_workloop_worker_thread + 590
    frame #22: 0x00000001099dfb84 libsystem_pthread.dylib`_pthread_wqthread + 327
    frame #23: 0x00000001099deacf libsystem_pthread.dylib`start_wqthread + 15
```

## Changelog:

[IOS] [FIXED] - Fixes main thread stuck when reload in bridgeless mode

Pull Request resolved: #45486

Test Plan: RNTester, enables fabric, which is very easy to repro by tapping `r`  command multiple times quickly to trigger reload.

Reviewed By: philIip

Differential Revision: D59911929

Pulled By: cipolleschi

fbshipit-source-id: e7e431a11d26c399fa767b6cbf45e16bce24b9a0
  • Loading branch information
@zhongwuzw @facebook-github-bot
zhongwuzw authored and facebook-github-bot committed Aug 2, 2024
1 parent 878e1f3 commit a778979
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 68 deletions.
4 changes: 2 additions & 2 deletions packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ RuntimeExecutor ReactInstance::getUnbufferedRuntimeExecutor() noexcept {

// This BufferedRuntimeExecutor ensures that the main JS bundle finished
// execution before any JS queued into it from C++ are executed. Use
// getBufferedRuntimeExecutor() instead if you do not need the main JS bundle to
// have finished. e.g. setting global variables into JS runtime.
// getUnbufferedRuntimeExecutor() instead if you do not need the main JS bundle
// to have finished. e.g. setting global variables into JS runtime.
RuntimeExecutor ReactInstance::getBufferedRuntimeExecutor() noexcept {
return [weakBufferedRuntimeExecutor_ =
std::weak_ptr<BufferedRuntimeExecutor>(bufferedRuntimeExecutor_)](
Expand Down
52 changes: 2 additions & 50 deletions packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,6 @@ @implementation RCTHost {

NSDictionary *_launchOptions;

// All the surfaces that need to be started after main bundle execution
NSMutableArray<RCTFabricSurface *> *_surfaceStartBuffer;
std::mutex _surfaceStartBufferMutex;

RCTInstanceInitialBundleLoadCompletionBlock _onInitialBundleLoad;
std::vector<__weak RCTFabricSurface *> _attachedSurfaces;

RCTModuleRegistry *_moduleRegistry;
Expand Down Expand Up @@ -152,7 +147,6 @@ - (instancetype)initWithBundleURLProvider:(RCTHostBundleURLProvider)provider
if (self = [super init]) {
_hostDelegate = hostDelegate;
_turboModuleManagerDelegate = turboModuleManagerDelegate;
_surfaceStartBuffer = [NSMutableArray new];
_bundleManager = [RCTBundleManager new];
_moduleRegistry = [RCTModuleRegistry new];
_jsEngineProvider = [jsEngineProvider copy];
Expand Down Expand Up @@ -185,33 +179,6 @@ - (instancetype)initWithBundleURLProvider:(RCTHostBundleURLProvider)provider
andSetter:bundleURLSetter
andDefaultGetter:defaultBundleURLGetter];

/**
* TODO (T108401473) Remove _onInitialBundleLoad, because it was initially
* introduced to start surfaces after the main JSBundle was fully executed.
* It is no longer needed because Fabric now dispatches all native-to-JS calls
* onto the JS thread, including JS calls to start Fabric surfaces.
* JS calls should be dispatched using the BufferedRuntimeExecutor, which can buffer
* JS calls before the main JSBundle finishes execution, and execute them after.
*/
_onInitialBundleLoad = ^{
RCTHost *strongSelf = weakSelf;
if (!strongSelf) {
return;
}

NSArray<RCTFabricSurface *> *unstartedSurfaces = @[];

{
std::lock_guard<std::mutex> guard{strongSelf->_surfaceStartBufferMutex};
unstartedSurfaces = [NSArray arrayWithArray:strongSelf->_surfaceStartBuffer];
strongSelf->_surfaceStartBuffer = nil;
}

for (RCTFabricSurface *surface in unstartedSurfaces) {
[surface start];
}
};

// Listen to reload commands
dispatch_async(dispatch_get_main_queue(), ^{
RCTRegisterReloadCommandListener(self);
Expand Down Expand Up @@ -261,7 +228,6 @@ - (void)start
jsRuntimeFactory:[self _provideJSEngine]
bundleManager:_bundleManager
turboModuleManagerDelegate:_turboModuleManagerDelegate
onInitialBundleLoad:_onInitialBundleLoad
moduleRegistry:_moduleRegistry
parentInspectorTarget:_inspectorTarget.get()
launchOptions:_launchOptions];
Expand All @@ -278,15 +244,7 @@ - (RCTFabricSurface *)createSurfaceWithModuleName:(NSString *)moduleName
surface.surfaceHandler.setDisplayMode(displayMode);
[self _attachSurface:surface];

{
std::lock_guard<std::mutex> guard{_surfaceStartBufferMutex};
if (_surfaceStartBuffer) {
[_surfaceStartBuffer addObject:surface];
return surface;
}
}

[surface start];
[_instance callFunctionOnBufferedRumtimeExecutor:[surface](facebook::jsi::Runtime &_) { [surface start]; }];
return surface;
}

Expand Down Expand Up @@ -320,24 +278,18 @@ - (void)didReceiveReloadCommand
[self _setBundleURL:_bundleURLProvider()];
}

// Ensure all attached surfaces are restarted after reload
{
std::lock_guard<std::mutex> guard{_surfaceStartBufferMutex};
_surfaceStartBuffer = [NSMutableArray arrayWithArray:[self _getAttachedSurfaces]];
}

_instance = [[RCTInstance alloc] initWithDelegate:self
jsRuntimeFactory:[self _provideJSEngine]
bundleManager:_bundleManager
turboModuleManagerDelegate:_turboModuleManagerDelegate
onInitialBundleLoad:_onInitialBundleLoad
moduleRegistry:_moduleRegistry
parentInspectorTarget:_inspectorTarget.get()
launchOptions:_launchOptions];
[_hostDelegate hostDidStart:self];

for (RCTFabricSurface *surface in [self _getAttachedSurfaces]) {
[surface resetWithSurfacePresenter:self.surfacePresenter];
[_instance callFunctionOnBufferedRumtimeExecutor:[surface](facebook::jsi::Runtime &_) { [surface start]; }];
}
}

Expand Down
4 changes: 1 addition & 3 deletions packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ RCT_EXTERN void RCTInstanceSetRuntimeDiagnosticFlags(NSString *_Nullable flags);

@end

typedef void (^_Null_unspecified RCTInstanceInitialBundleLoadCompletionBlock)();

/**
* RCTInstance owns and manages most of the pieces of infrastructure required to display a screen powered by React
* Native. RCTInstance should never be instantiated in product code, but rather accessed through RCTHost. The host
Expand All @@ -59,12 +57,12 @@ typedef void (^_Null_unspecified RCTInstanceInitialBundleLoadCompletionBlock)();
jsRuntimeFactory:(std::shared_ptr<facebook::react::JSRuntimeFactory>)jsRuntimeFactory
bundleManager:(RCTBundleManager *)bundleManager
turboModuleManagerDelegate:(id<RCTTurboModuleManagerDelegate>)turboModuleManagerDelegate
onInitialBundleLoad:(RCTInstanceInitialBundleLoadCompletionBlock)onInitialBundleLoad
moduleRegistry:(RCTModuleRegistry *)moduleRegistry
parentInspectorTarget:(facebook::react::jsinspector_modern::HostTarget *)parentInspectorTarget
launchOptions:(nullable NSDictionary *)launchOptions;

- (void)callFunctionOnJSModule:(NSString *)moduleName method:(NSString *)method args:(NSArray *)args;
- (void)callFunctionOnBufferedRumtimeExecutor:(std::function<void(facebook::jsi::Runtime &runtime)> &&)executor;

- (void)registerSegmentWithId:(NSNumber *)segmentId path:(NSString *)path;

Expand Down
17 changes: 9 additions & 8 deletions packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ @implementation RCTInstance {
RCTSurfacePresenter *_surfacePresenter;
RCTPerformanceLogger *_performanceLogger;
RCTDisplayLink *_displayLink;
RCTInstanceInitialBundleLoadCompletionBlock _onInitialBundleLoad;
RCTTurboModuleManager *_turboModuleManager;
std::mutex _invalidationMutex;
std::atomic<bool> _valid;
Expand All @@ -97,7 +96,6 @@ - (instancetype)initWithDelegate:(id<RCTInstanceDelegate>)delegate
jsRuntimeFactory:(std::shared_ptr<facebook::react::JSRuntimeFactory>)jsRuntimeFactory
bundleManager:(RCTBundleManager *)bundleManager
turboModuleManagerDelegate:(id<RCTTurboModuleManagerDelegate>)tmmDelegate
onInitialBundleLoad:(RCTInstanceInitialBundleLoadCompletionBlock)onInitialBundleLoad
moduleRegistry:(RCTModuleRegistry *)moduleRegistry
parentInspectorTarget:(jsinspector_modern::HostTarget *)parentInspectorTarget
launchOptions:(nullable NSDictionary *)launchOptions
Expand All @@ -111,7 +109,6 @@ - (instancetype)initWithDelegate:(id<RCTInstanceDelegate>)delegate
_jsRuntimeFactory = jsRuntimeFactory;
_appTMMDelegate = tmmDelegate;
_jsThreadManager = [RCTJSThreadManager new];
_onInitialBundleLoad = onInitialBundleLoad;
_bridgeModuleDecorator = [[RCTBridgeModuleDecorator alloc] initWithViewRegistry:[RCTViewRegistry new]
moduleRegistry:moduleRegistry
bundleManager:bundleManager
Expand Down Expand Up @@ -393,6 +390,15 @@ - (void)_attachBridgelessAPIsToModule:(id<RCTTurboModule>)module
}
}

- (void)callFunctionOnBufferedRumtimeExecutor:(std::function<void(facebook::jsi::Runtime &)> &&)executor
{
_reactInstance->getBufferedRuntimeExecutor()([=](jsi::Runtime &runtime) {
if (executor) {
executor(runtime);
}
});
}

- (void)handleBundleLoadingError:(NSError *)error
{
if (!_valid) {
Expand Down Expand Up @@ -467,11 +473,6 @@ - (void)_loadScriptFromSource:(RCTSource *)source
const auto *url = deriveSourceURL(source.url).UTF8String;
_reactInstance->loadScript(std::move(script), url);
[[NSNotificationCenter defaultCenter] postNotificationName:@"RCTInstanceDidLoadBundle" object:nil];

if (_onInitialBundleLoad) {
_onInitialBundleLoad();
_onInitialBundleLoad = nil;
}
}

- (void)_handleJSError:(const JsErrorHandler::ParsedError &)error
Expand Down
9 changes: 4 additions & 5 deletions packages/react-native/ReactCommon/react/test_utils/ios/Shims/ShimRCTInstance.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ - (instancetype)init
[RCTInstance class],
[ShimRCTInstance class],
@selector(initWithDelegate:
jsRuntimeFactory:bundleManager:turboModuleManagerDelegate:onInitialBundleLoad:moduleRegistry
:parentInspectorTarget:launchOptions:));
jsRuntimeFactory:bundleManager:turboModuleManagerDelegate:moduleRegistry:parentInspectorTarget
:launchOptions:));
RCTSwizzleInstanceSelector([RCTInstance class], [ShimRCTInstance class], @selector(invalidate));
RCTSwizzleInstanceSelector(
[RCTInstance class], [ShimRCTInstance class], @selector(callFunctionOnJSModule:method:args:));
Expand All @@ -39,8 +39,8 @@ - (void)reset
[RCTInstance class],
[ShimRCTInstance class],
@selector(initWithDelegate:
jsRuntimeFactory:bundleManager:turboModuleManagerDelegate:onInitialBundleLoad:moduleRegistry
:parentInspectorTarget:launchOptions:));
jsRuntimeFactory:bundleManager:turboModuleManagerDelegate:moduleRegistry:parentInspectorTarget
:launchOptions:));
RCTSwizzleInstanceSelector([RCTInstance class], [ShimRCTInstance class], @selector(invalidate));
RCTSwizzleInstanceSelector(
[RCTInstance class], [ShimRCTInstance class], @selector(callFunctionOnJSModule:method:args:));
Expand All @@ -52,7 +52,6 @@ - (instancetype)initWithDelegate:(id<RCTInstanceDelegate>)delegate
jsRuntimeFactory:(std::shared_ptr<facebook::react::JSRuntimeFactory>)jsRuntimeFactory
bundleManager:(RCTBundleManager *)bundleManager
turboModuleManagerDelegate:(id<RCTTurboModuleManagerDelegate>)tmmDelegate
onInitialBundleLoad:(RCTInstanceInitialBundleLoadCompletionBlock)onInitialBundleLoad
moduleRegistry:(RCTModuleRegistry *)moduleRegistry
parentInspectorTarget:(facebook::react::jsinspector_modern::HostTarget *)parentInspectorTarget
launchOptions:(NSDictionary *)launchOptions
Expand Down

0 comments on commit a778979

Please sign in to comment.