From 23cd9febbc63b633f8ef8be25387df21c1c52b7e Mon Sep 17 00:00:00 2001 From: Nick Lockwood Date: Thu, 7 Jan 2016 01:29:43 -0800 Subject: [PATCH] Fixed deadlock in RCTModuleData Summary: public Fixed a potential deadlock issue if code attempted to access a module via [bridge moduleForName/Class:] while it was being initialized. Reviewed By: lry Differential Revision: D2807827 fb-gh-sync-id: 58cafe9b92c094dde632d17245fb9b342a0fe9e0 --- React/Base/RCTBatchedBridge.m | 9 +-------- React/Base/RCTModuleData.h | 9 ++++++++- React/Base/RCTModuleData.m | 26 ++++++++++++++++---------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/React/Base/RCTBatchedBridge.m b/React/Base/RCTBatchedBridge.m index 185d6379e..e619c67d7 100644 --- a/React/Base/RCTBatchedBridge.m +++ b/React/Base/RCTBatchedBridge.m @@ -333,14 +333,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); for (RCTModuleData *moduleData in _moduleDataByID) { if (moduleData.hasInstance) { - [moduleData methodQueue]; // initialize the queue - } - } - - for (RCTModuleData *moduleData in _moduleDataByID) { - if (moduleData.hasInstance) { - [self registerModuleForFrameUpdates:moduleData.instance - withModuleData:moduleData]; + [moduleData finishSetupForInstance]; } } diff --git a/React/Base/RCTModuleData.h b/React/Base/RCTModuleData.h index bf55892fd..ec922c804 100644 --- a/React/Base/RCTModuleData.h +++ b/React/Base/RCTModuleData.h @@ -25,11 +25,18 @@ /** * Sets the bridge for the module instance. This is only needed when using the - * `initWithModuleID:instance:` constructor. Otherwise, the bridge will be set + * `initWithModuleInstance:bridge:` constructor. Otherwise, the bridge will be set * automatically when the module is first accessed. */ - (void)setBridgeForInstance; +/** + * Sets the methodQueue and performs the remaining setup for the module. This is + * only needed when using the `initWithModuleInstance:bridge:` constructor. + * Otherwise it will be done automatically when the module is first accessed. + */ +- (void)finishSetupForInstance; + @property (nonatomic, strong, readonly) Class moduleClass; @property (nonatomic, copy, readonly) NSString *name; diff --git a/React/Base/RCTModuleData.m b/React/Base/RCTModuleData.m index 2e290ce6c..d87edf430 100644 --- a/React/Base/RCTModuleData.m +++ b/React/Base/RCTModuleData.m @@ -70,6 +70,18 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); } } +- (void)finishSetupForInstance +{ + if (!_setupComplete) { + _setupComplete = YES; + [self setUpMethodQueue]; + [_bridge registerModuleForFrameUpdates:_instance withModuleData:self]; + [[NSNotificationCenter defaultCenter] postNotificationName:RCTDidInitializeModuleNotification + object:_bridge + userInfo:@{@"module": _instance}]; + } +} + - (void)setUpMethodQueue { if (!_methodQueue) { @@ -90,19 +102,13 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); [(id)_instance setValue:_methodQueue forKey:@"methodQueue"]; } @catch (NSException *exception) { - RCTLogError(@"%@ is returning nil for it's methodQueue, which is not " + RCTLogError(@"%@ is returning nil for its methodQueue, which is not " "permitted. You must either return a pre-initialized " "queue, or @synthesize the methodQueue to let the bridge " "create a queue for you.", self.name); } } } - - // Needs to be sent after bridge has been set for all module instances. - // Makes sense to put it here, since the same rules apply for methodQueue. - [[NSNotificationCenter defaultCenter] - postNotificationName:RCTDidInitializeModuleNotification - object:_bridge userInfo:@{@"module": _instance}]; } } @@ -124,11 +130,11 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); // initialization requires it (View Managers get their queue by calling // self.bridge.uiManager.methodQueue) [self setBridgeForInstance]; - [self setUpMethodQueue]; - [_bridge registerModuleForFrameUpdates:_instance withModuleData:self]; - _setupComplete = YES; } [_instanceLock unlock]; + + [self finishSetupForInstance]; + return _instance; }