Skip to content

Commit ddc4225

Browse files
RSNarafacebook-github-bot
authored andcommitted
Move NativeModule initialization logic outside of setModuleRegistry
Summary: ## Context A while ago, we introduced a new initialization API in NativeModules via RCTInitializing.h (diff: D28435078 (9b45df1)). ## Problem A number of our NativeModules still use setModuleRegistry to perform initialization. ## Changes This diff migrates those NativeModules to instead use the initialize API. ## Motivation In bridgeless mode each NativeModule object is [created and decorated by the RCTInstance](https://www.internalfb.com/code/fbsource/[89f6c0df78e453a20555975e06bc46b4e0d2bbe9]/fbobjc/Apps/Internal/Venice/Core/RCTInstance.mm?lines=180-189), while [holding the TurboModuleManagerDelegate mutex](https://www.internalfb.com/code/fbsource/[c50ce2bb3fb078d28e1f9afdef5e8793f1413472]/xplat/js/react-native-github/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModuleManager.mm?lines=429%2C431). After D30753286, setModuleRegistry will be called in RCTInstance getModuleInstanceForClass, which means that we'll start calling setModuleRegistry while holding the TurboModuleManagerDelegate lock. This leads to a deadlock, because calling setModuleRegistry on RCTDeviceInfo [creates RCTAccessibilityManager](https://www.internalfb.com/code/fbsource/[89f6c0df78e453a20555975e06bc46b4e0d2bbe9]/xplat/js/react-native-github/React/CoreModules/RCTDeviceInfo.mm?lines=50), which tries to acquire the TurboModuleManagerDelegate lock again. The NativeModule initialize method isn't called while holding the TurboModuleManagerDelegate lock. That's why moving all initialization logic to the initialize method mitigates this deadlock hazard. In general, we shouldn't do any sort of initialization inside setters for these bridge/bridgeless APIs. No other NativeModules do initialization outside of initialize. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D30754870 fbshipit-source-id: 7c2d50f995cba6f58ee2dfebfabd36f640579bcb
1 parent 16a0930 commit ddc4225

File tree

5 files changed

+22
-14
lines changed

5 files changed

+22
-14
lines changed

Libraries/NativeAnimation/RCTNativeAnimatedModule.mm

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,17 @@
99
#import <React/RCTNativeAnimatedModule.h>
1010
#import <React/RCTNativeAnimatedNodesManager.h>
1111
#import <React/RCTLog.h>
12+
#import <React/RCTInitializing.h>
1213

1314
#import <RCTTypeSafety/RCTConvertHelpers.h>
1415

1516
#import "RCTAnimationPlugins.h"
1617

1718
typedef void (^AnimatedOperation)(RCTNativeAnimatedNodesManager *nodesManager);
1819

20+
@interface RCTNativeAnimatedModule () <RCTInitializing>
21+
@end
22+
1923
@implementation RCTNativeAnimatedModule
2024
{
2125
RCTNativeAnimatedNodesManager *_nodesManager;
@@ -69,10 +73,9 @@ - (void)setBridge:(RCTBridge *)bridge
6973
[bridge.surfacePresenter addObserver:self];
7074
}
7175

72-
- (void)setModuleRegistry:(RCTModuleRegistry *)moduleRegistry
76+
- (void)initialize
7377
{
74-
[super setModuleRegistry:moduleRegistry];
75-
[[moduleRegistry moduleForName:"EventDispatcher"] addDispatchObserver:self];
78+
[[self.moduleRegistry moduleForName:"EventDispatcher"] addDispatchObserver:self];
7679
}
7780

7881
/*

Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@
1212
#import <React/RCTUIManager.h>
1313
#import <React/RCTUIManagerObserverCoordinator.h>
1414
#import <React/RCTUIManagerUtils.h>
15-
#import <React/RCTInitializing.h>
1615

1716
#import "RCTValueAnimatedNode.h"
1817

1918
// TODO T69437152 @petetheheat - Delete this fork when Fabric ships to 100%.
2019
// NOTE: This module is temporarily forked (see RCTNativeAnimatedModule).
2120
// When making any changes, be sure to apply them to the fork as well.
22-
@interface RCTNativeAnimatedTurboModule: RCTEventEmitter <RCTBridgeModule, RCTValueAnimatedNodeObserver, RCTEventDispatcherObserver, RCTUIManagerObserver, RCTSurfacePresenterObserver, RCTInitializing>
21+
@interface RCTNativeAnimatedTurboModule: RCTEventEmitter <RCTBridgeModule, RCTValueAnimatedNodeObserver, RCTEventDispatcherObserver, RCTUIManagerObserver, RCTSurfacePresenterObserver>
2322

2423
@end

Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99
#import <RCTTypeSafety/RCTConvertHelpers.h>
1010
#import <React/RCTNativeAnimatedTurboModule.h>
1111
#import <React/RCTNativeAnimatedNodesManager.h>
12+
#import <React/RCTInitializing.h>
1213

1314
#import "RCTAnimationPlugins.h"
1415

1516
typedef void (^AnimatedOperation)(RCTNativeAnimatedNodesManager *nodesManager);
1617

17-
@interface RCTNativeAnimatedTurboModule() <NativeAnimatedModuleSpec>
18+
@interface RCTNativeAnimatedTurboModule() <NativeAnimatedModuleSpec, RCTInitializing>
1819
@end
1920

2021
@implementation RCTNativeAnimatedTurboModule

React/CoreModules/RCTDeviceInfo.mm

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@
1212
#import <React/RCTAssert.h>
1313
#import <React/RCTConstants.h>
1414
#import <React/RCTEventDispatcherProtocol.h>
15+
#import <React/RCTInitializing.h>
1516
#import <React/RCTUIUtils.h>
1617
#import <React/RCTUtils.h>
1718

1819
#import "CoreModulesPlugins.h"
1920

2021
using namespace facebook::react;
2122

22-
@interface RCTDeviceInfo () <NativeDeviceInfoSpec>
23+
@interface RCTDeviceInfo () <NativeDeviceInfoSpec, RCTInitializing>
2324
@end
2425

2526
@implementation RCTDeviceInfo {
@@ -41,13 +42,12 @@ - (dispatch_queue_t)methodQueue
4142
return dispatch_get_main_queue();
4243
}
4344

44-
- (void)setModuleRegistry:(RCTModuleRegistry *)moduleRegistry
45+
- (void)initialize
4546
{
46-
_moduleRegistry = moduleRegistry;
4747
[[NSNotificationCenter defaultCenter] addObserver:self
4848
selector:@selector(didReceiveNewContentSizeMultiplier)
4949
name:RCTAccessibilityManagerDidUpdateMultiplierNotification
50-
object:[moduleRegistry moduleForName:"AccessibilityManager"]];
50+
object:[_moduleRegistry moduleForName:"AccessibilityManager"]];
5151

5252
_currentInterfaceOrientation = [RCTSharedApplication() statusBarOrientation];
5353

React/CoreModules/RCTPerfMonitor.mm

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#import <React/RCTBridge+Private.h>
2121
#import <React/RCTBridge.h>
2222
#import <React/RCTFPSGraph.h>
23+
#import <React/RCTInitializing.h>
2324
#import <React/RCTInvalidating.h>
2425
#import <React/RCTJavaScriptExecutor.h>
2526
#import <React/RCTPerformanceLogger.h>
@@ -84,8 +85,13 @@ static vm_size_t RCTGetResidentMemorySize(void)
8485
return memoryUsageInByte;
8586
}
8687

87-
@interface RCTPerfMonitor
88-
: NSObject <RCTBridgeModule, RCTTurboModule, RCTInvalidating, UITableViewDataSource, UITableViewDelegate>
88+
@interface RCTPerfMonitor : NSObject <
89+
RCTBridgeModule,
90+
RCTTurboModule,
91+
RCTInitializing,
92+
RCTInvalidating,
93+
UITableViewDataSource,
94+
UITableViewDelegate>
8995

9096
#if __has_include(<React/RCTDevMenu.h>)
9197
@property (nonatomic, strong, readonly) RCTDevMenuItem *devMenuItem;
@@ -150,9 +156,8 @@ - (dispatch_queue_t)methodQueue
150156
return dispatch_get_main_queue();
151157
}
152158

153-
- (void)setModuleRegistry:(RCTModuleRegistry *)moduleRegistry
159+
- (void)initialize
154160
{
155-
_moduleRegistry = moduleRegistry;
156161
#if __has_include(<React/RCTDevMenu.h>)
157162
[(RCTDevMenu *)[_moduleRegistry moduleForName:"DevMenu"] addItem:self.devMenuItem];
158163
#endif

0 commit comments

Comments
 (0)