From a778979ed604d1dc7707faf4a3836e3deba136c6 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Fri, 2 Aug 2024 13:48:31 -0700 Subject: [PATCH] Fixes main thread stuck when reload in bridgeless mode (#45486) Summary: In fabric bridgeless mode, when we reload, main thread may block because of dead lock. the backtrace example as below: ``` (lldb) bt * thread https://github.com/facebook/react-native/issues/1, stop reason = signal SIGSTOP * frame #0: 0x000000010a5c76f2 libsystem_kernel.dylib`__psynch_mutexwait + 10 frame https://github.com/facebook/react-native/issues/1: 0x00000001099e0a70 libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_wait + 78 frame https://github.com/facebook/react-native/issues/2: 0x00000001099de82b libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_slow + 217 frame https://github.com/facebook/react-native/issues/3: 0x00007ff80030c6b9 libc++.1.dylib`std::__1::mutex::lock() + 9 frame https://github.com/facebook/react-native/issues/4: 0x0000000106968b13 RNTester`std::__1::lock_guard::lock_guard[abi:ue170006](this=0x00007ff7b95e2478, __m=0x000060000377c958) at lock_guard.h:35:10 frame https://github.com/facebook/react-native/issues/5: 0x00000001069689ed RNTester`std::__1::lock_guard::lock_guard[abi:ue170006](this=0x00007ff7b95e2478, __m=0x000060000377c958) at lock_guard.h:34:19 frame https://github.com/facebook/react-native/issues/6: 0x00000001070691c1 RNTester`-[RCTInstance invalidate](self=0x000060000377c900, _cmd="invalidate") at RCTInstance.mm:146:31 frame https://github.com/facebook/react-native/issues/7: 0x0000000107060fd2 RNTester`-[RCTHost didReceiveReloadCommand](self=0x0000600003d100f0, _cmd="didReceiveReloadCommand") at RCTHost.mm:317:3 frame https://github.com/facebook/react-native/issues/8: 0x0000000106b005a5 RNTester`RCTTriggerReloadCommandListeners(reason=@"Global hotkey") at RCTReloadCommand.m:57:5 frame https://github.com/facebook/react-native/issues/9: 0x0000000106b86da5 RNTester`__28-[RCTDevSettings initialize]_block_invoke.157(.block_descriptor=0x0000000107496170, params=0x00007ff84002f610) at RCTDevSettings.mm:201:11 frame https://github.com/facebook/react-native/issues/10: 0x0000000106ae658e RNTester`__65-[RCTPackagerConnection reconnectingWebSocket:didReceiveMessage:]_block_invoke.68(.block_descriptor=0x0000600000c82df0) at RCTPackagerConnection.mm:293:9 frame https://github.com/facebook/react-native/issues/11: 0x0000000109a4529d libdispatch.dylib`_dispatch_call_block_and_release + 12 frame https://github.com/facebook/react-native/issues/12: 0x0000000109a4658f libdispatch.dylib`_dispatch_client_callout + 8 frame https://github.com/facebook/react-native/issues/13: 0x0000000109a563ee libdispatch.dylib`_dispatch_main_queue_drain + 1362 frame https://github.com/facebook/react-native/issues/14: 0x0000000109a55e8e libdispatch.dylib`_dispatch_main_queue_callback_4CF + 31 frame https://github.com/facebook/react-native/issues/15: 0x00007ff800429af4 CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9 frame https://github.com/facebook/react-native/issues/16: 0x00007ff80042442f CoreFoundation`__CFRunLoopRun + 2463 frame https://github.com/facebook/react-native/issues/17: 0x00007ff8004236ad CoreFoundation`CFRunLoopRunSpecific + 557 frame https://github.com/facebook/react-native/issues/18: 0x00007ff8103da08f GraphicsServices`GSEventRunModal + 137 frame https://github.com/facebook/react-native/issues/19: 0x00007ff805cc0ad1 UIKitCore`-[UIApplication _run] + 972 frame https://github.com/facebook/react-native/issues/20: 0x00007ff805cc5551 UIKitCore`UIApplicationMain + 123 frame https://github.com/facebook/react-native/issues/21: 0x00000001069205a0 RNTester`main(argc=1, argv=0x00007ff7b95e3b60) at main.m:15:12 frame https://github.com/facebook/react-native/issues/22: 0x00000001099023e0 dyld_sim`start_sim + 10 frame https://github.com/facebook/react-native/issues/23: 0x0000000116b92366 dyld`start + 1942 (lldb) bt * thread https://github.com/facebook/react-native/issues/3 frame #0: 0x000000010a5c6b86 libsystem_kernel.dylib`__ulock_wait + 10 frame https://github.com/facebook/react-native/issues/1: 0x0000000109a46eb1 libdispatch.dylib`_dlock_wait + 46 frame https://github.com/facebook/react-native/issues/2: 0x0000000109a46d08 libdispatch.dylib`_dispatch_thread_event_wait_slow + 40 frame https://github.com/facebook/react-native/issues/3: 0x0000000109a5774a libdispatch.dylib`__DISPATCH_WAIT_FOR_QUEUE__ + 371 frame https://github.com/facebook/react-native/issues/4: 0x0000000109a57161 libdispatch.dylib`_dispatch_sync_f_slow + 240 frame https://github.com/facebook/react-native/issues/5: 0x0000000106b3b33b RNTester`RCTUnsafeExecuteOnMainQueueSync(block=0x0000000106f116c0) at RCTUtils.m:291:5 * frame https://github.com/facebook/react-native/issues/6: 0x0000000106f115ad RNTester`-[RCTFabricSurface start](self=0x000000010af0df40, _cmd="start") at RCTFabricSurface.mm:102:3 frame https://github.com/facebook/react-native/issues/7: 0x00000001070601ce RNTester`__108-[RCTHost initWithBundleURLProvider:hostDelegate:turboModuleManagerDelegate:jsEngineProvider:launchOptions:]_block_invoke_2(.block_descriptor=0x0000600000c75590) at RCTHost.mm:211:9 frame https://github.com/facebook/react-native/issues/8: 0x000000010706cdc8 RNTester`-[RCTInstance _loadScriptFromSource:](self=0x000060000377c900, _cmd="_loadScriptFromSource:", source=0x0000600000cd57d0) at RCTInstance.mm:472:5 frame https://github.com/facebook/react-native/issues/9: 0x000000010706ca81 RNTester`__29-[RCTInstance _loadJSBundle:]_block_invoke.120(.block_descriptor=0x0000600000c96d00, error=0x0000000000000000, source=0x0000600000cd57d0) at RCTInstance.mm:452:9 frame https://github.com/facebook/react-native/issues/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 https://github.com/facebook/react-native/issues/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 https://github.com/facebook/react-native/issues/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 https://github.com/facebook/react-native/issues/13: 0x0000000106ada800 RNTester`-[RCTMultipartStreamReader readAllPartsWithCompletionCallback:progressCallback:](self=0x00006000002b4220, _cmd="readAllPartsWithCompletionCallback:progressCallback:", callback=0x0000000106ad9230, progressCallback=0x0000000106ab2a60) at RCTMultipartStreamReader.m:154:7 frame https://github.com/facebook/react-native/issues/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 https://github.com/facebook/react-native/issues/15: 0x00007ff80479fdf9 CFNetwork`___lldb_unnamed_symbol2876 + 42 frame https://github.com/facebook/react-native/issues/16: 0x0000000109a4529d libdispatch.dylib`_dispatch_call_block_and_release + 12 frame https://github.com/facebook/react-native/issues/17: 0x0000000109a4658f libdispatch.dylib`_dispatch_client_callout + 8 frame https://github.com/facebook/react-native/issues/18: 0x0000000109a4e4ba libdispatch.dylib`_dispatch_lane_serial_drain + 1127 frame https://github.com/facebook/react-native/issues/19: 0x0000000109a4f255 libdispatch.dylib`_dispatch_lane_invoke + 441 frame https://github.com/facebook/react-native/issues/20: 0x0000000109a5c356 libdispatch.dylib`_dispatch_root_queue_drain_deferred_wlh + 318 frame https://github.com/facebook/react-native/issues/21: 0x0000000109a5b751 libdispatch.dylib`_dispatch_workloop_worker_thread + 590 frame https://github.com/facebook/react-native/issues/22: 0x00000001099dfb84 libsystem_pthread.dylib`_pthread_wqthread + 327 frame https://github.com/facebook/react-native/issues/23: 0x00000001099deacf libsystem_pthread.dylib`start_wqthread + 15 ``` ## Changelog: [IOS] [FIXED] - Fixes main thread stuck when reload in bridgeless mode Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../react/runtime/ReactInstance.cpp | 4 +- .../platform/ios/ReactCommon/RCTHost.mm | 52 +------------------ .../platform/ios/ReactCommon/RCTInstance.h | 4 +- .../platform/ios/ReactCommon/RCTInstance.mm | 17 +++--- .../test_utils/ios/Shims/ShimRCTInstance.mm | 9 ++-- 5 files changed, 18 insertions(+), 68 deletions(-) diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index b305ada5813264..041c774919c3ca 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -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_)]( diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm index 9fdac6566a5bed..914a8e8d037518 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHost.mm @@ -104,11 +104,6 @@ @implementation RCTHost { NSDictionary *_launchOptions; - // All the surfaces that need to be started after main bundle execution - NSMutableArray *_surfaceStartBuffer; - std::mutex _surfaceStartBufferMutex; - - RCTInstanceInitialBundleLoadCompletionBlock _onInitialBundleLoad; std::vector<__weak RCTFabricSurface *> _attachedSurfaces; RCTModuleRegistry *_moduleRegistry; @@ -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]; @@ -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 *unstartedSurfaces = @[]; - - { - std::lock_guard 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); @@ -261,7 +228,6 @@ - (void)start jsRuntimeFactory:[self _provideJSEngine] bundleManager:_bundleManager turboModuleManagerDelegate:_turboModuleManagerDelegate - onInitialBundleLoad:_onInitialBundleLoad moduleRegistry:_moduleRegistry parentInspectorTarget:_inspectorTarget.get() launchOptions:_launchOptions]; @@ -278,15 +244,7 @@ - (RCTFabricSurface *)createSurfaceWithModuleName:(NSString *)moduleName surface.surfaceHandler.setDisplayMode(displayMode); [self _attachSurface:surface]; - { - std::lock_guard guard{_surfaceStartBufferMutex}; - if (_surfaceStartBuffer) { - [_surfaceStartBuffer addObject:surface]; - return surface; - } - } - - [surface start]; + [_instance callFunctionOnBufferedRumtimeExecutor:[surface](facebook::jsi::Runtime &_) { [surface start]; }]; return surface; } @@ -320,17 +278,10 @@ - (void)didReceiveReloadCommand [self _setBundleURL:_bundleURLProvider()]; } - // Ensure all attached surfaces are restarted after reload - { - std::lock_guard 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]; @@ -338,6 +289,7 @@ - (void)didReceiveReloadCommand for (RCTFabricSurface *surface in [self _getAttachedSurfaces]) { [surface resetWithSurfacePresenter:self.surfacePresenter]; + [_instance callFunctionOnBufferedRumtimeExecutor:[surface](facebook::jsi::Runtime &_) { [surface start]; }]; } } diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.h b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.h index 4f94b5700b5154..359bfa34743677 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.h @@ -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 @@ -59,12 +57,12 @@ typedef void (^_Null_unspecified RCTInstanceInitialBundleLoadCompletionBlock)(); jsRuntimeFactory:(std::shared_ptr)jsRuntimeFactory bundleManager:(RCTBundleManager *)bundleManager turboModuleManagerDelegate:(id)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 &&)executor; - (void)registerSegmentWithId:(NSNumber *)segmentId path:(NSString *)path; diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm index ddeb1e9eff9437..6872139d876234 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm @@ -78,7 +78,6 @@ @implementation RCTInstance { RCTSurfacePresenter *_surfacePresenter; RCTPerformanceLogger *_performanceLogger; RCTDisplayLink *_displayLink; - RCTInstanceInitialBundleLoadCompletionBlock _onInitialBundleLoad; RCTTurboModuleManager *_turboModuleManager; std::mutex _invalidationMutex; std::atomic _valid; @@ -97,7 +96,6 @@ - (instancetype)initWithDelegate:(id)delegate jsRuntimeFactory:(std::shared_ptr)jsRuntimeFactory bundleManager:(RCTBundleManager *)bundleManager turboModuleManagerDelegate:(id)tmmDelegate - onInitialBundleLoad:(RCTInstanceInitialBundleLoadCompletionBlock)onInitialBundleLoad moduleRegistry:(RCTModuleRegistry *)moduleRegistry parentInspectorTarget:(jsinspector_modern::HostTarget *)parentInspectorTarget launchOptions:(nullable NSDictionary *)launchOptions @@ -111,7 +109,6 @@ - (instancetype)initWithDelegate:(id)delegate _jsRuntimeFactory = jsRuntimeFactory; _appTMMDelegate = tmmDelegate; _jsThreadManager = [RCTJSThreadManager new]; - _onInitialBundleLoad = onInitialBundleLoad; _bridgeModuleDecorator = [[RCTBridgeModuleDecorator alloc] initWithViewRegistry:[RCTViewRegistry new] moduleRegistry:moduleRegistry bundleManager:bundleManager @@ -393,6 +390,15 @@ - (void)_attachBridgelessAPIsToModule:(id)module } } +- (void)callFunctionOnBufferedRumtimeExecutor:(std::function &&)executor +{ + _reactInstance->getBufferedRuntimeExecutor()([=](jsi::Runtime &runtime) { + if (executor) { + executor(runtime); + } + }); +} + - (void)handleBundleLoadingError:(NSError *)error { if (!_valid) { @@ -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 diff --git a/packages/react-native/ReactCommon/react/test_utils/ios/Shims/ShimRCTInstance.mm b/packages/react-native/ReactCommon/react/test_utils/ios/Shims/ShimRCTInstance.mm index 2575f1453a2953..3b9947000e58db 100644 --- a/packages/react-native/ReactCommon/react/test_utils/ios/Shims/ShimRCTInstance.mm +++ b/packages/react-native/ReactCommon/react/test_utils/ios/Shims/ShimRCTInstance.mm @@ -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:)); @@ -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:)); @@ -52,7 +52,6 @@ - (instancetype)initWithDelegate:(id)delegate jsRuntimeFactory:(std::shared_ptr)jsRuntimeFactory bundleManager:(RCTBundleManager *)bundleManager turboModuleManagerDelegate:(id)tmmDelegate - onInitialBundleLoad:(RCTInstanceInitialBundleLoadCompletionBlock)onInitialBundleLoad moduleRegistry:(RCTModuleRegistry *)moduleRegistry parentInspectorTarget:(facebook::react::jsinspector_modern::HostTarget *)parentInspectorTarget launchOptions:(NSDictionary *)launchOptions