Skip to content

Commit

Permalink
Avoid retaining TurboModuleManager in AppDelegate (#37104)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #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: 416105a3fdf03ed0b4f7330c5ba5fd7fe7be21c2
  • Loading branch information
javache authored and facebook-github-bot committed Apr 26, 2023
1 parent 2cda4f7 commit a5860f2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 17 deletions.
4 changes: 0 additions & 4 deletions packages/react-native/Libraries/AppDelegate/RCTAppDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#import <UIKit/UIKit.h>

@class RCTSurfacePresenterBridgeAdapter;
@class RCTTurboModuleManager;

/**
* The RCTAppDelegate is an utility class that implements some base configurations for all the React Native apps.
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,12 @@ - (UIViewController *)createRootViewController
_runtimeScheduler = std::make_shared<facebook::react::RuntimeScheduler>(RCTRuntimeExecutorFromBridge(bridge));
std::shared_ptr<facebook::react::CallInvoker> callInvoker =
std::make_shared<facebook::react::RuntimeSchedulerCallInvoker>(_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
Expand Down
23 changes: 12 additions & 11 deletions packages/rn-tester/RNTester/AppDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ @interface AppDelegate () <RCTCxxBridgeDelegate, RCTTurboModuleManagerDelegate>
facebook::react::ContextContainer::Shared _contextContainer;
std::shared_ptr<facebook::react::RuntimeScheduler> _runtimeScheduler;
#endif

RCTTurboModuleManager *_turboModuleManager;
}
@end

Expand Down Expand Up @@ -207,15 +205,18 @@ - (void)loadSourceForBridge:(RCTBridge *)bridge
_contextContainer->insert("RuntimeScheduler", _runtimeScheduler);
callInvoker = std::make_shared<facebook::react::RuntimeSchedulerCallInvoker>(_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;
Expand All @@ -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<void(facebook::jsi::Runtime & runtime_)> &&callback) { callback(runtime); };
[strongSelf->_turboModuleManager installJSBindingWithRuntimeExecutor:syncRuntimeExecutor];
}

facebook::react::RuntimeExecutor syncRuntimeExecutor =
[&](std::function<void(facebook::jsi::Runtime & runtime_)> &&callback) { callback(runtime); };
[turboModuleManager installJSBindingWithRuntimeExecutor:syncRuntimeExecutor];
}));
}

Expand Down

0 comments on commit a5860f2

Please sign in to comment.