From 6e32378efaca6f367e899609d22dedac71c54f82 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Wed, 17 Jul 2024 10:23:48 +0800 Subject: [PATCH 1/3] [iOS] Fixes main thread stuck when reload in bridgeless mode --- .../react/runtime/platform/ios/ReactCommon/RCTInstance.mm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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..560d5a95765c12 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 @@ -469,8 +469,10 @@ - (void)_loadScriptFromSource:(RCTSource *)source [[NSNotificationCenter defaultCenter] postNotificationName:@"RCTInstanceDidLoadBundle" object:nil]; if (_onInitialBundleLoad) { - _onInitialBundleLoad(); - _onInitialBundleLoad = nil; + auto onInitialBundleLoad = _onInitialBundleLoad; + RCTExecuteOnMainQueue(^{ + onInitialBundleLoad(); + }); } } From 586c91f21a640db501bf8a046bf19a45ccadc944 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Wed, 17 Jul 2024 16:45:45 +0800 Subject: [PATCH 2/3] Add missed nil set --- .../react/runtime/platform/ios/ReactCommon/RCTInstance.mm | 1 + 1 file changed, 1 insertion(+) 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 560d5a95765c12..ffc333f06eef65 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 @@ -473,6 +473,7 @@ - (void)_loadScriptFromSource:(RCTSource *)source RCTExecuteOnMainQueue(^{ onInitialBundleLoad(); }); + _onInitialBundleLoad = nil; } } From ff282ebb835295ead8b05d16577a9baac94e1317 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Tue, 23 Jul 2024 20:25:17 +0800 Subject: [PATCH 3/3] Call start surface on bufferedRuntime --- .../react/runtime/ReactInstance.cpp | 4 +- .../platform/ios/ReactCommon/RCTHost.mm | 54 ++----------------- .../platform/ios/ReactCommon/RCTInstance.h | 1 + .../platform/ios/ReactCommon/RCTInstance.mm | 14 +++-- 4 files changed, 17 insertions(+), 56 deletions(-) diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 0b8128c586f4b0..380199cc1ad3ff 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -148,8 +148,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..d8c472dc39af40 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,7 @@ - (void)start jsRuntimeFactory:[self _provideJSEngine] bundleManager:_bundleManager turboModuleManagerDelegate:_turboModuleManagerDelegate - onInitialBundleLoad:_onInitialBundleLoad + onInitialBundleLoad:nil moduleRegistry:_moduleRegistry parentInspectorTarget:_inspectorTarget.get() launchOptions:_launchOptions]; @@ -278,15 +245,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 +279,11 @@ - (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 + onInitialBundleLoad:nil moduleRegistry:_moduleRegistry parentInspectorTarget:_inspectorTarget.get() launchOptions:_launchOptions]; @@ -338,6 +291,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..2694e58bc46ec3 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 @@ -65,6 +65,7 @@ typedef void (^_Null_unspecified RCTInstanceInitialBundleLoadCompletionBlock)(); 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 ffc333f06eef65..cb78fb540d2ba8 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 @@ -393,6 +393,15 @@ - (void)_attachBridgelessAPIsToModule:(id)module } } +- (void)callFunctionOnBufferedRumtimeExecutor:(std::function &&)executor +{ + self->_reactInstance->getBufferedRuntimeExecutor()([=](jsi::Runtime &runtime) { + if (executor) { + executor(runtime); + } + }); +} + - (void)handleBundleLoadingError:(NSError *)error { if (!_valid) { @@ -469,10 +478,7 @@ - (void)_loadScriptFromSource:(RCTSource *)source [[NSNotificationCenter defaultCenter] postNotificationName:@"RCTInstanceDidLoadBundle" object:nil]; if (_onInitialBundleLoad) { - auto onInitialBundleLoad = _onInitialBundleLoad; - RCTExecuteOnMainQueue(^{ - onInitialBundleLoad(); - }); + _onInitialBundleLoad(); _onInitialBundleLoad = nil; } }