From c32731216b4e366256e09b7c6b796c69267b0d27 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 26 Apr 2023 08:38:27 -0700 Subject: [PATCH] Avoid retaining TurboModuleManager in AppDelegate (#37104) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/37104 The AppDelegate (and other similar classes) only need to construct the TurboModuleManager for intialization purposes, and should not retain it beyond that. TurboModuleManager retains each of the TurboModule instances and through the new binding mechanism, also JSI pointers, which are invalid beyond the lifetime of the JS context. Changelog: [iOS] Changed AppDelegate template to avoid retaining TurboModuleManager. Reviewed By: sammy-SC, cipolleschi Differential Revision: D45310066 fbshipit-source-id: 8f369ecd980332acbd4c08d872c9296233221991 --- .../Libraries/AppDelegate/RCTAppDelegate.h | 4 ---- .../Libraries/AppDelegate/RCTAppDelegate.mm | 6 +++-- packages/rn-tester/RNTester/AppDelegate.mm | 23 ++++++++++--------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h index 18c318408d8c90..93b1543c7bacc4 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h +++ b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h @@ -10,7 +10,6 @@ #import @class RCTSurfacePresenterBridgeAdapter; -@class RCTTurboModuleManager; /** * The RCTAppDelegate is an utility class that implements some base configurations for all the React Native apps. @@ -95,9 +94,6 @@ - (UIViewController *)createRootViewController; #if RCT_NEW_ARCH_ENABLED - -/// The TurboModule manager -@property (nonatomic, strong) RCTTurboModuleManager *turboModuleManager; @property (nonatomic, strong) RCTSurfacePresenterBridgeAdapter *bridgeAdapter; /// This method controls whether the `turboModules` feature of the New Architecture is turned on or off. diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm index 4d04efa280eab8..5548c7d0605d09 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm @@ -130,10 +130,12 @@ - (UIViewController *)createRootViewController _runtimeScheduler = std::make_shared(RCTRuntimeExecutorFromBridge(bridge)); std::shared_ptr callInvoker = std::make_shared(_runtimeScheduler); - self.turboModuleManager = [[RCTTurboModuleManager alloc] initWithBridge:bridge delegate:self jsInvoker:callInvoker]; + RCTTurboModule *turboModuleManager = [[RCTTurboModuleManager alloc] initWithBridge:bridge + delegate:self + jsInvoker:callInvoker]; _contextContainer->erase("RuntimeScheduler"); _contextContainer->insert("RuntimeScheduler", _runtimeScheduler); - return RCTAppSetupDefaultJsExecutorFactory(bridge, _turboModuleManager, _runtimeScheduler); + return RCTAppSetupDefaultJsExecutorFactory(bridge, turboModuleManager, _runtimeScheduler); } #pragma mark - RCTTurboModuleManagerDelegate diff --git a/packages/rn-tester/RNTester/AppDelegate.mm b/packages/rn-tester/RNTester/AppDelegate.mm index b8bf7a2b54dbdd..b9b7e2aac40459 100644 --- a/packages/rn-tester/RNTester/AppDelegate.mm +++ b/packages/rn-tester/RNTester/AppDelegate.mm @@ -82,8 +82,6 @@ @interface AppDelegate () facebook::react::ContextContainer::Shared _contextContainer; std::shared_ptr _runtimeScheduler; #endif - - RCTTurboModuleManager *_turboModuleManager; } @end @@ -207,15 +205,18 @@ - (void)loadSourceForBridge:(RCTBridge *)bridge _contextContainer->insert("RuntimeScheduler", _runtimeScheduler); callInvoker = std::make_shared(_runtimeScheduler); #endif - _turboModuleManager = [[RCTTurboModuleManager alloc] initWithBridge:bridge delegate:self jsInvoker:callInvoker]; - [bridge setRCTTurboModuleRegistry:_turboModuleManager]; + + RCTTurboModuleManager *turboModuleManager = [[RCTTurboModuleManager alloc] initWithBridge:bridge + delegate:self + jsInvoker:callInvoker]; + [bridge setRCTTurboModuleRegistry:turboModuleManager]; #if RCT_DEV /** * Eagerly initialize RCTDevMenu so CMD + d, CMD + i, and CMD + r work. * This is a stop gap until we have a system to eagerly init Turbo Modules. */ - [_turboModuleManager moduleForName:"RCTDevMenu"]; + [turboModuleManager moduleForName:"RCTDevMenu"]; #endif __weak __typeof(self) weakSelf = self; @@ -229,17 +230,17 @@ - (void)loadSourceForBridge:(RCTBridge *)bridge if (!bridge) { return; } - __typeof(self) strongSelf = weakSelf; + #if RN_FABRIC_ENABLED + __typeof(self) strongSelf = weakSelf; if (strongSelf && strongSelf->_runtimeScheduler) { facebook::react::RuntimeSchedulerBinding::createAndInstallIfNeeded(runtime, strongSelf->_runtimeScheduler); } #endif - if (strongSelf) { - facebook::react::RuntimeExecutor syncRuntimeExecutor = - [&](std::function &&callback) { callback(runtime); }; - [strongSelf->_turboModuleManager installJSBindingWithRuntimeExecutor:syncRuntimeExecutor]; - } + + facebook::react::RuntimeExecutor syncRuntimeExecutor = + [&](std::function &&callback) { callback(runtime); }; + [turboModuleManager installJSBindingWithRuntimeExecutor:syncRuntimeExecutor]; })); }