From 9228873fb4b8161ba7370a56712ba121804b14b1 Mon Sep 17 00:00:00 2001 From: Tadeu Zagallo Date: Mon, 22 Jun 2015 04:55:55 -0700 Subject: [PATCH] [ReactNative] Fix racing conditions on reload Summary: @public That was eventually being released before all the queues had been cleared. Update it so the each modules' queue is immediately invalidated after sending the `-invalidate` message to it, and introduce an intentional retain cycle so the bridge is only released together with all modules, when all the messages have been dispatched. Test Plan: Launch the UIExplorer, and reload it, like, a lot. --- React/Base/RCTBridge.m | 53 ++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index a50079440..d7ec0ac48 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -1042,11 +1042,7 @@ RCT_INNER_BRIDGE_ONLY(_invokeAndProcessModule:(__unused NSString *)module - (NSDictionary *)modules { - if (!self.isValid) { - return nil; - } - - RCTAssert(_modulesByName != nil, @"Bridge modules have not yet been initialized. " + RCTAssert(!self.isValid || _modulesByName != nil, @"Bridge modules have not yet been initialized. " "You may be trying to access a module too early in the startup procedure."); return _modulesByName; @@ -1074,19 +1070,21 @@ RCT_INNER_BRIDGE_ONLY(_invokeAndProcessModule:(__unused NSString *)module _mainDisplayLink = nil; // Invalidate modules + dispatch_group_t group = dispatch_group_create(); for (id target in _modulesByID.allObjects) { if ([target respondsToSelector:@selector(invalidate)]) { [self dispatchBlock:^{ [(id)target invalidate]; - } forModule:target]; + } forModule:target dispatchGroup:group]; } + _queuesByID[RCTModuleIDsByName[RCTBridgeModuleNameForClass([target class])]] = nil; } - - // Release modules (breaks retain cycle if module has strong bridge reference) - _frameUpdateObservers = nil; - _modulesByID = nil; - _queuesByID = nil; - _modulesByName = nil; + dispatch_group_notify(group, dispatch_get_main_queue(), ^{ + _queuesByID = nil; + _modulesByID = nil; + _modulesByName = nil; + _frameUpdateObservers = nil; + }); }; if (!_javaScriptExecutor) { @@ -1185,13 +1183,30 @@ RCT_INNER_BRIDGE_ONLY(_invokeAndProcessModule:(__unused NSString *)module } #pragma mark - Payload Generation - -- (void)dispatchBlock:(dispatch_block_t)block forModule:(id)module +- (void)dispatchBlock:(dispatch_block_t)block + forModule:(id)module { - [self dispatchBlock:block forModuleID:RCTModuleIDsByName[RCTBridgeModuleNameForClass([module class])]]; + [self dispatchBlock:block forModule:module dispatchGroup:NULL]; } -- (void)dispatchBlock:(dispatch_block_t)block forModuleID:(NSNumber *)moduleID +- (void)dispatchBlock:(dispatch_block_t)block + forModule:(id)module + dispatchGroup:(dispatch_group_t)group +{ + [self dispatchBlock:block + forModuleID:RCTModuleIDsByName[RCTBridgeModuleNameForClass([module class])] + dispatchGroup:group]; +} + +- (void)dispatchBlock:(dispatch_block_t)block + forModuleID:(NSNumber *)moduleID +{ + [self dispatchBlock:block forModuleID:moduleID dispatchGroup:NULL]; +} + +- (void)dispatchBlock:(dispatch_block_t)block + forModuleID:(NSNumber *)moduleID + dispatchGroup:(dispatch_group_t)group { RCTAssertJSThread(); @@ -1203,7 +1218,11 @@ RCT_INNER_BRIDGE_ONLY(_invokeAndProcessModule:(__unused NSString *)module if (queue == RCTJSThread) { [_javaScriptExecutor executeBlockOnJavaScriptQueue:block]; } else if (queue) { - dispatch_async(queue, block); + if (group != NULL) { + dispatch_group_async(group, queue, block); + } else { + dispatch_async(queue, block); + } } }