From dc13115445681c1e04079f6a47668e1f12e6020a Mon Sep 17 00:00:00 2001 From: Nick Lockwood Date: Thu, 3 Mar 2016 02:20:20 -0800 Subject: [PATCH] Dispatch module setup asynchronously to avoid blocking main thread when bridge starts Summary:Initializing native modules can block the main thread for tens of milliseconds when it starts up, making it difficult to instantiate the bridge on demand without causing a performance blip. This diff splits up the initialization of modules so that - although they still happen on the main thread - they don't block the thread continuously. Reviewed By: javache Differential Revision: D2965438 fb-gh-sync-id: 38c9c9d281e4672b5874d68b57d4c60d1d268344 shipit-source-id: 38c9c9d281e4672b5874d68b57d4c60d1d268344 --- .../UIExplorerUnitTests/RCTImageLoaderTests.m | 20 +- .../RCTModuleInitNotificationRaceTests.m | 15 +- .../UIExplorerUnitTests/RCTModuleInitTests.m | 9 +- Libraries/Image/RCTImageLoader.h | 9 + Libraries/Image/RCTImageLoader.m | 82 +++--- Libraries/Network/RCTNetworking.m | 15 +- React/Base/RCTBatchedBridge.m | 246 +++++++++--------- React/Base/RCTBridge+Private.h | 5 + React/Base/RCTBridge.h | 37 +-- React/Base/RCTBridge.m | 53 +--- React/Base/RCTModuleData.h | 26 +- React/Base/RCTModuleData.m | 109 ++++++-- React/Views/RCTViewManager.m | 3 + 13 files changed, 311 insertions(+), 318 deletions(-) diff --git a/Examples/UIExplorer/UIExplorerUnitTests/RCTImageLoaderTests.m b/Examples/UIExplorer/UIExplorerUnitTests/RCTImageLoaderTests.m index 5713d0347..5a7cc12bd 100644 --- a/Examples/UIExplorer/UIExplorerUnitTests/RCTImageLoaderTests.m +++ b/Examples/UIExplorer/UIExplorerUnitTests/RCTImageLoaderTests.m @@ -47,10 +47,9 @@ RCTDefineImageDecoder(RCTImageLoaderTestsDecoder2) return nil; }]; - RCTImageLoader *imageLoader = [RCTImageLoader new]; - NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[loader, imageLoader]; } launchOptions:nil]; + NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[loader]; } launchOptions:nil]; - [imageLoader loadImageWithTag:@"http://facebook.github.io/react/img/logo_og.png" size:CGSizeMake(100, 100) scale:1.0 resizeMode:RCTResizeModeContain progressBlock:^(int64_t progress, int64_t total) { + [bridge.imageLoader loadImageWithTag:@"http://facebook.github.io/react/img/logo_og.png" size:CGSizeMake(100, 100) scale:1.0 resizeMode:RCTResizeModeContain progressBlock:^(int64_t progress, int64_t total) { XCTAssertEqual(progress, 1); XCTAssertEqual(total, 1); } completionBlock:^(NSError *loadError, id loadedImage) { @@ -78,10 +77,9 @@ RCTDefineImageDecoder(RCTImageLoaderTestsDecoder2) return nil; }]; - RCTImageLoader *imageLoader = [RCTImageLoader new]; - NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[loader1, loader2, imageLoader]; } launchOptions:nil]; + NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[loader1, loader2]; } launchOptions:nil]; - [imageLoader loadImageWithTag:@"http://facebook.github.io/react/img/logo_og.png" size:CGSizeMake(100, 100) scale:1.0 resizeMode:RCTResizeModeContain progressBlock:^(int64_t progress, int64_t total) { + [bridge.imageLoader loadImageWithTag:@"http://facebook.github.io/react/img/logo_og.png" size:CGSizeMake(100, 100) scale:1.0 resizeMode:RCTResizeModeContain progressBlock:^(int64_t progress, int64_t total) { XCTAssertEqual(progress, 1); XCTAssertEqual(total, 1); } completionBlock:^(NSError *loadError, id loadedImage) { @@ -103,10 +101,9 @@ RCTDefineImageDecoder(RCTImageLoaderTestsDecoder2) return nil; }]; - RCTImageLoader *imageLoader = [RCTImageLoader new]; - NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[decoder, imageLoader]; } launchOptions:nil]; + NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[decoder]; } launchOptions:nil]; - RCTImageLoaderCancellationBlock cancelBlock = [imageLoader decodeImageData:data size:CGSizeMake(1, 1) scale:1.0 resizeMode:RCTResizeModeStretch completionBlock:^(NSError *decodeError, id decodedImage) { + RCTImageLoaderCancellationBlock cancelBlock = [bridge.imageLoader decodeImageDataWithoutClipping:data size:CGSizeMake(1, 1) scale:1.0 resizeMode:RCTResizeModeStretch completionBlock:^(NSError *decodeError, id decodedImage) { XCTAssertEqualObjects(decodedImage, image); XCTAssertNil(decodeError); }]; @@ -133,10 +130,9 @@ RCTDefineImageDecoder(RCTImageLoaderTestsDecoder2) return nil; }]; - RCTImageLoader *imageLoader = [RCTImageLoader new]; - NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[decoder1, decoder2, imageLoader]; } launchOptions:nil]; + NS_VALID_UNTIL_END_OF_SCOPE RCTBridge *bridge = [[RCTBridge alloc] initWithBundleURL:nil moduleProvider:^{ return @[decoder1, decoder2]; } launchOptions:nil]; - RCTImageLoaderCancellationBlock cancelBlock = [imageLoader decodeImageData:data size:CGSizeMake(1, 1) scale:1.0 resizeMode:RCTResizeModeStretch completionBlock:^(NSError *decodeError, id decodedImage) { + RCTImageLoaderCancellationBlock cancelBlock = [bridge.imageLoader decodeImageDataWithoutClipping:data size:CGSizeMake(1, 1) scale:1.0 resizeMode:RCTResizeModeStretch completionBlock:^(NSError *decodeError, id decodedImage) { XCTAssertEqualObjects(decodedImage, image); XCTAssertNil(decodeError); }]; diff --git a/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitNotificationRaceTests.m b/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitNotificationRaceTests.m index 39650db27..77a997a4b 100644 --- a/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitNotificationRaceTests.m +++ b/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitNotificationRaceTests.m @@ -34,26 +34,13 @@ } \ } -// Must be declared before RCTTestCustomSetBridgeModule in order to trigger the -// race condition that we are testing for - namely that the -// RCTDidInitializeModuleNotification for RCTTestViewManager gets sent before -// setBridge: is called on RCTTestCustomSetBridgeModule @interface RCTTestViewManager : RCTViewManager @end @implementation RCTTestViewManager -@synthesize bridge = _bridge; -@synthesize methodQueue = _methodQueue; - RCT_EXPORT_MODULE() -- (void)setBridge:(RCTBridge *)bridge -{ - _bridge = bridge; - (void)[_bridge uiManager]; // Needed to trigger a race condition -} - - (NSArray *)customDirectEventTypes { return @[@"foo"]; @@ -112,7 +99,7 @@ RCT_EXPORT_MODULE() - (NSArray *)extraModulesForBridge:(__unused RCTBridge *)bridge { - return @[_notificationObserver]; + return @[[RCTTestViewManager new], _notificationObserver]; } - (void)setUp diff --git a/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitTests.m b/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitTests.m index a0289b2e4..83a549736 100644 --- a/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitTests.m +++ b/Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitTests.m @@ -202,10 +202,11 @@ RCT_EXPORT_MODULE() - (void)testInjectedModulesInitializedDuringBridgeInit { - XCTAssertTrue(_injectedModuleInitNotificationSent); XCTAssertEqual(_injectedModule, [_bridge moduleForClass:[RCTTestInjectedModule class]]); XCTAssertEqual(_injectedModule.bridge, _bridge.batchedBridge); XCTAssertNotNil(_injectedModule.methodQueue); + RUN_RUNLOOP_WHILE(!_injectedModuleInitNotificationSent); + XCTAssertTrue(_injectedModuleInitNotificationSent); } - (void)testCustomInitModuleInitializedAtBridgeStartup @@ -214,6 +215,8 @@ RCT_EXPORT_MODULE() XCTAssertTrue(_customInitModuleNotificationSent); RCTTestCustomInitModule *module = [_bridge moduleForClass:[RCTTestCustomInitModule class]]; XCTAssertTrue(module.initializedOnMainThread); + XCTAssertEqual(module.bridge, _bridge.batchedBridge); + XCTAssertNotNil(module.methodQueue); } - (void)testCustomSetBridgeModuleInitializedAtBridgeStartup @@ -222,6 +225,8 @@ RCT_EXPORT_MODULE() XCTAssertTrue(_customSetBridgeModuleNotificationSent); RCTTestCustomSetBridgeModule *module = [_bridge moduleForClass:[RCTTestCustomSetBridgeModule class]]; XCTAssertTrue(module.setBridgeOnMainThread); + XCTAssertEqual(module.bridge, _bridge.batchedBridge); + XCTAssertNotNil(module.methodQueue); } - (void)testExportConstantsModuleInitializedAtBridgeStartup @@ -232,6 +237,8 @@ RCT_EXPORT_MODULE() RUN_RUNLOOP_WHILE(!module.exportedConstants); XCTAssertTrue(module.exportedConstants); XCTAssertTrue(module.exportedConstantsOnMainThread); + XCTAssertEqual(module.bridge, _bridge.batchedBridge); + XCTAssertNotNil(module.methodQueue); } - (void)testLazyInitModuleNotInitializedDuringBridgeInit diff --git a/Libraries/Image/RCTImageLoader.h b/Libraries/Image/RCTImageLoader.h index cfe3d83fc..d11c42410 100644 --- a/Libraries/Image/RCTImageLoader.h +++ b/Libraries/Image/RCTImageLoader.h @@ -91,6 +91,15 @@ typedef void (^RCTImageLoaderCancellationBlock)(void); resizeMode:(RCTResizeMode)resizeMode completionBlock:(RCTImageLoaderCompletionBlock)completionBlock; +/** + * Decodes an image without clipping the result to fit. + */ +- (RCTImageLoaderCancellationBlock)decodeImageDataWithoutClipping:(NSData *)data + size:(CGSize)size + scale:(CGFloat)scale + resizeMode:(RCTResizeMode)resizeMode + completionBlock:(RCTImageLoaderCompletionBlock)completionBlock; + /** * Get image size, in pixels. This method will do the least work possible to get * the information, and won't decode the image if it doesn't have to. diff --git a/Libraries/Image/RCTImageLoader.m b/Libraries/Image/RCTImageLoader.m index ea4d42acf..207ecf702 100644 --- a/Libraries/Image/RCTImageLoader.m +++ b/Libraries/Image/RCTImageLoader.m @@ -59,54 +59,31 @@ RCT_EXPORT_MODULE() _maxConcurrentDecodingTasks = _maxConcurrentDecodingTasks ?: 2; _maxConcurrentDecodingBytes = _maxConcurrentDecodingBytes ?: 30 * 1024 *1024; // 30MB - // Get image loaders and decoders - NSMutableArray> *loaders = [NSMutableArray array]; - NSMutableArray> *decoders = [NSMutableArray array]; - for (Class moduleClass in _bridge.moduleClasses) { - if ([moduleClass conformsToProtocol:@protocol(RCTImageURLLoader)]) { - [loaders addObject:[_bridge moduleForClass:moduleClass]]; - } - if ([moduleClass conformsToProtocol:@protocol(RCTImageDataDecoder)]) { - [decoders addObject:[_bridge moduleForClass:moduleClass]]; - } - } - - // Sort loaders in reverse priority order (highest priority first) - [loaders sortUsingComparator:^NSComparisonResult(id a, id b) { - float priorityA = [a respondsToSelector:@selector(loaderPriority)] ? [a loaderPriority] : 0; - float priorityB = [b respondsToSelector:@selector(loaderPriority)] ? [b loaderPriority] : 0; - if (priorityA > priorityB) { - return NSOrderedAscending; - } else if (priorityA < priorityB) { - return NSOrderedDescending; - } else { - return NSOrderedSame; - } - }]; - - // Sort decoders in reverse priority order (highest priority first) - [decoders sortUsingComparator:^NSComparisonResult(id a, id b) { - float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0; - float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0; - if (priorityA > priorityB) { - return NSOrderedAscending; - } else if (priorityA < priorityB) { - return NSOrderedDescending; - } else { - return NSOrderedSame; - } - }]; - - _loaders = loaders; - _decoders = decoders; + _URLCacheQueue = dispatch_queue_create("com.facebook.react.ImageLoaderURLCacheQueue", DISPATCH_QUEUE_SERIAL); } - (id)imageURLLoaderForURL:(NSURL *)URL { - if (!_loaders) { + if (!_maxConcurrentLoadingTasks) { [self setUp]; } + if (!_loaders) { + // Get loaders, sorted in reverse priority order (highest priority first) + RCTAssert(_bridge, @"Bridge not set"); + _loaders = [[_bridge modulesConformingToProtocol:@protocol(RCTImageURLLoader)] sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { + float priorityA = [a respondsToSelector:@selector(loaderPriority)] ? [a loaderPriority] : 0; + float priorityB = [b respondsToSelector:@selector(loaderPriority)] ? [b loaderPriority] : 0; + if (priorityA > priorityB) { + return NSOrderedAscending; + } else if (priorityA < priorityB) { + return NSOrderedDescending; + } else { + return NSOrderedSame; + } + }]; + } + if (RCT_DEBUG) { // Check for handler conflicts float previousPriority = 0; @@ -144,10 +121,26 @@ RCT_EXPORT_MODULE() - (id)imageDataDecoderForData:(NSData *)data { - if (!_decoders) { + if (!_maxConcurrentLoadingTasks) { [self setUp]; } + if (!_decoders) { + // Get decoders, sorted in reverse priority order (highest priority first) + RCTAssert(_bridge, @"Bridge not set"); + _decoders = [[_bridge modulesConformingToProtocol:@protocol(RCTImageDataDecoder)] sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { + float priorityA = [a respondsToSelector:@selector(decoderPriority)] ? [a decoderPriority] : 0; + float priorityB = [b respondsToSelector:@selector(decoderPriority)] ? [b decoderPriority] : 0; + if (priorityA > priorityB) { + return NSOrderedAscending; + } else if (priorityA < priorityB) { + return NSOrderedDescending; + } else { + return NSOrderedSame; + } + }]; + } + if (RCT_DEBUG) { // Check for handler conflicts float previousPriority = 0; @@ -295,7 +288,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, // All access to URL cache must be serialized if (!_URLCacheQueue) { - _URLCacheQueue = dispatch_queue_create("com.facebook.react.ImageLoaderURLCacheQueue", DISPATCH_QUEUE_SERIAL); + [self setUp]; } dispatch_async(_URLCacheQueue, ^{ @@ -539,6 +532,9 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, completionHandler:completionHandler]; } else { + if (!_URLCacheQueue) { + [self setUp]; + } dispatch_async(_URLCacheQueue, ^{ dispatch_block_t decodeBlock = ^{ diff --git a/Libraries/Network/RCTNetworking.m b/Libraries/Network/RCTNetworking.m index 47cbc74d2..88c2de0e0 100644 --- a/Libraries/Network/RCTNetworking.m +++ b/Libraries/Network/RCTNetworking.m @@ -141,17 +141,8 @@ RCT_EXPORT_MODULE() } if (!_handlers) { - - // get handlers - NSMutableArray> *handlers = [NSMutableArray array]; - for (Class moduleClass in _bridge.moduleClasses) { - if ([moduleClass conformsToProtocol:@protocol(RCTURLRequestHandler)]) { - [handlers addObject:[_bridge moduleForClass:moduleClass]]; - } - } - - // Sort handlers in reverse priority order (highest priority first) - [handlers sortUsingComparator:^NSComparisonResult(id a, id b) { + // Get handlers, sorted in reverse priority order (highest priority first) + _handlers = [[_bridge modulesConformingToProtocol:@protocol(RCTURLRequestHandler)] sortedArrayUsingComparator:^NSComparisonResult(id a, id b) { float priorityA = [a respondsToSelector:@selector(handlerPriority)] ? [a handlerPriority] : 0; float priorityB = [b respondsToSelector:@selector(handlerPriority)] ? [b handlerPriority] : 0; if (priorityA > priorityB) { @@ -162,8 +153,6 @@ RCT_EXPORT_MODULE() return NSOrderedSame; } }]; - - _handlers = handlers; } if (RCT_DEBUG) { diff --git a/React/Base/RCTBatchedBridge.m b/React/Base/RCTBatchedBridge.m index 3dd44ec9e..3154e4813 100644 --- a/React/Base/RCTBatchedBridge.m +++ b/React/Base/RCTBatchedBridge.m @@ -45,29 +45,25 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); @property (nonatomic, weak) RCTBridge *parentBridge; @property (nonatomic, weak) id javaScriptExecutor; +@property (nonatomic, assign) BOOL moduleSetupComplete; @end @implementation RCTBatchedBridge { - BOOL _loading; - BOOL _valid; BOOL _wasBatchActive; NSMutableArray *_pendingCalls; NSMutableDictionary *_moduleDataByName; NSArray *_moduleDataByID; - NSDictionary> *_modulesByName_DEPRECATED; NSArray *_moduleClassesByID; CADisplayLink *_jsDisplayLink; NSMutableSet *_frameUpdateObservers; - - // Bridge startup stats (TODO: capture in perf logger) - NSUInteger _syncInitializedModules; - NSUInteger _asyncInitializedModules; } @synthesize flowID = _flowID; @synthesize flowIDMap = _flowIDMap; +@synthesize loading = _loading; +@synthesize valid = _valid; - (instancetype)initWithParentBridge:(RCTBridge *)bridge { @@ -122,11 +118,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); }]; // Synchronously initialize all native modules that cannot be loaded lazily - [self initModules]; - -#if RCT_DEBUG - _syncInitializedModules = [[_moduleDataByID valueForKeyPath:@"@sum.hasInstance"] integerValue]; -#endif + [self initModulesWithDispatchGroup:initModulesAndLoadSource]; if (RCTProfileIsProfiling()) { // Depends on moduleDataByID being loaded @@ -145,17 +137,10 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); // Asynchronously gather the module config dispatch_group_async(setupJSExecutorAndModuleConfig, bridgeQueue, ^{ - if (weakSelf.isValid) { - + if (weakSelf.valid) { RCTPerformanceLoggerStart(RCTPLNativeModulePrepareConfig); config = [weakSelf moduleConfig]; RCTPerformanceLoggerEnd(RCTPLNativeModulePrepareConfig); - -#if RCT_DEBUG - NSInteger total = [[_moduleDataByID valueForKeyPath:@"@sum.hasInstance"] integerValue]; - _asyncInitializedModules = total - _syncInitializedModules; -#endif - } }); @@ -217,7 +202,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); - (NSArray *)moduleClasses { - if (RCT_DEBUG && self.isValid && _moduleClassesByID == nil) { + if (RCT_DEBUG && _valid && _moduleClassesByID == nil) { RCTLogError(@"Bridge modules have not yet been initialized. You may be " "trying to access a module too early in the startup procedure."); } @@ -234,8 +219,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); - (id)moduleForName:(NSString *)moduleName { - RCTModuleData *moduleData = _moduleDataByName[moduleName]; - return moduleData.instance; + return _moduleDataByName[moduleName].instance; } - (NSArray *)configForModuleName:(NSString *)moduleName @@ -250,14 +234,11 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); return (id)kCFNull; } -- (void)initModules +- (void)initModulesWithDispatchGroup:(dispatch_group_t)dispatchGroup { RCTAssertMainThread(); RCTPerformanceLoggerStart(RCTPLNativeModuleInit); - // Register passed-in module instances - NSMutableDictionary *preregisteredModules = [NSMutableDictionary new]; - NSArray> *extraModules = nil; if (self.delegate) { if ([self.delegate respondsToSelector:@selector(extraModulesForBridge:)]) { @@ -267,95 +248,133 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); extraModules = self.moduleProvider(); } - for (id module in extraModules) { - preregisteredModules[RCTBridgeModuleNameForClass([module class])] = module; + if (RCT_DEBUG && !RCTRunningInTestEnvironment()) { + + // Check for unexported modules + static Class *classes; + static unsigned int classCount; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + classes = objc_copyClassList(&classCount); + }); + + NSMutableSet *moduleClasses = [NSMutableSet new]; + [moduleClasses addObjectsFromArray:RCTGetModuleClasses()]; + [moduleClasses addObjectsFromArray:[extraModules valueForKeyPath:@"class"]]; + + for (unsigned int i = 0; i < classCount; i++) + { + Class cls = classes[i]; + Class superclass = cls; + while (superclass) + { + if (class_conformsToProtocol(superclass, @protocol(RCTBridgeModule))) + { + if (![moduleClasses containsObject:cls]) { + RCTLogWarn(@"Class %@ was not exported. Did you forget to use " + "RCT_EXPORT_MODULE()?", cls); + } + break; + } + superclass = class_getSuperclass(superclass); + } + } } - SEL setBridgeSelector = NSSelectorFromString(@"setBridge:"); - IMP objectInitMethod = [NSObject instanceMethodForSelector:@selector(init)]; - - // Set up moduleData and pre-initialize module instances + NSMutableArray *moduleClassesByID = [NSMutableArray new]; NSMutableArray *moduleDataByID = [NSMutableArray new]; NSMutableDictionary *moduleDataByName = [NSMutableDictionary new]; + + // Set up moduleData for pre-initialized module instances + for (id module in extraModules) { + Class moduleClass = [module class]; + NSString *moduleName = RCTBridgeModuleNameForClass(moduleClass); + + if (RCT_DEBUG) { + // Check for name collisions between preregistered modules + RCTModuleData *moduleData = moduleDataByName[moduleName]; + if (moduleData) { + RCTLogError(@"Attempted to register RCTBridgeModule class %@ for the " + "name '%@', but name was already registered by class %@", + moduleClass, moduleName, moduleData.moduleClass); + continue; + } + } + + // Instantiate moduleData container + RCTModuleData *moduleData = [[RCTModuleData alloc] initWithModuleInstance:module + bridge:self]; + moduleDataByName[moduleName] = moduleData; + [moduleClassesByID addObject:moduleClass]; + [moduleDataByID addObject:moduleData]; + } + + // Set up moduleData for automatically-exported modules for (Class moduleClass in RCTGetModuleClasses()) { NSString *moduleName = RCTBridgeModuleNameForClass(moduleClass); - id module = preregisteredModules[moduleName]; - if (!module) { - // Check if the module class, or any of its superclasses override init - // or setBridge:, or has exported constants. If they do, we assume that - // they are expecting to be initialized when the bridge first loads. - if ([moduleClass instanceMethodForSelector:@selector(init)] != objectInitMethod || - [moduleClass instancesRespondToSelector:setBridgeSelector] || - RCTClassOverridesInstanceMethod(moduleClass, @selector(constantsToExport))) { - module = [moduleClass new]; - if (!module) { - module = (id)kCFNull; - } - } - } - // Check for module name collisions. - // It's OK to have a name collision as long as the second instance is null. - if (module != (id)kCFNull && moduleDataByName[moduleName] && !preregisteredModules[moduleName]) { - RCTLogError(@"Attempted to register RCTBridgeModule class %@ for the name " - "'%@', but name was already registered by class %@", moduleClass, - moduleName, moduleDataByName[moduleName].moduleClass); - } - - // Instantiate moduleData (TODO: defer this until config generation) - RCTModuleData *moduleData; - if (module) { - if (module != (id)kCFNull) { - moduleData = [[RCTModuleData alloc] initWithModuleInstance:module - bridge:self]; - } - } else { - moduleData = [[RCTModuleData alloc] initWithModuleClass:moduleClass - bridge:self]; - } + // Check for module name collisions + RCTModuleData *moduleData = moduleDataByName[moduleName]; if (moduleData) { - moduleDataByName[moduleName] = moduleData; - [moduleDataByID addObject:moduleData]; + if (moduleData.hasInstance) { + // Existing module was preregistered, so it takes precedence + continue; + } else if ([moduleClass new] == nil) { + // The new module returned nil from init, so use the old module + continue; + } else if ([moduleData.moduleClass new] != nil) { + // Both modules were non-nil, so it's unclear which should take precedence + RCTLogError(@"Attempted to register RCTBridgeModule class %@ for the " + "name '%@', but name was already registered by class %@", + moduleClass, moduleName, moduleData.moduleClass); + } } + + // Instantiate moduleData (TODO: can we defer this until config generation?) + moduleData = [[RCTModuleData alloc] initWithModuleClass:moduleClass + bridge:self]; + moduleDataByName[moduleName] = moduleData; + [moduleClassesByID addObject:moduleClass]; + [moduleDataByID addObject:moduleData]; } // Store modules _moduleDataByID = [moduleDataByID copy]; _moduleDataByName = [moduleDataByName copy]; - _moduleClassesByID = [moduleDataByID valueForKey:@"moduleClass"]; + _moduleClassesByID = [moduleClassesByID copy]; - /** - * The executor is a bridge module, wait for it to be created and set it before - * any other module has access to the bridge - */ + // The executor is a bridge module, wait for it to be created and set it up + // before any other module has access to the bridge. _javaScriptExecutor = [self moduleForClass:self.executorClass]; + // Dispatch module init onto main thead for those modules that require it for (RCTModuleData *moduleData in _moduleDataByID) { if (moduleData.hasInstance) { - [moduleData setBridgeForInstance]; + // Modules that were pre-initialized need to be set up before bridge init + // has finished, otherwise the caller may try to access the module + // directly rather than via `[bridge moduleForClass:]`, which won't + // trigger the lazy initialization process. + (void)[moduleData instance]; + } + if (moduleData.requiresMainThreadSetup) { + // Modules that need to be set up on the main thread cannot be initialized + // lazily when required without doing a dispatch_sync to the main thread, + // which can result in deadlock. To avoid this, we initialize all of these + // modules on the main thread in parallel with loading the JS code, so that + // they will already be available before they are ever required. + __weak RCTBatchedBridge *weakSelf = self; + dispatch_group_async(dispatchGroup, dispatch_get_main_queue(), ^{ + if (weakSelf.valid) { + (void)[moduleData instance]; + [moduleData gatherConstants]; + } + }); } } - for (RCTModuleData *moduleData in _moduleDataByID) { - if (moduleData.hasInstance) { - [moduleData finishSetupForInstance]; - } - } - - for (RCTModuleData *moduleData in _moduleDataByID) { - if (moduleData.hasInstance) { - [moduleData gatherConstants]; - } - } - -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" - - [[NSNotificationCenter defaultCenter] - postNotificationName:RCTDidCreateNativeModules - object:self userInfo:@{@"bridge": self}]; - -#pragma clang diagnostic pop + // From this point on, RCTDidInitializeModuleNotification notifications will + // be sent the first time a module is accessed. + _moduleSetupComplete = YES; RCTPerformanceLoggerEnd(RCTPLNativeModuleInit); } @@ -418,7 +437,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); - (void)injectJSONConfiguration:(NSString *)configJSON onComplete:(void (^)(NSError *))onComplete { - if (!self.valid) { + if (!_valid) { return; } @@ -429,7 +448,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); - (void)executeSourceCode:(NSData *)sourceCode { - if (!self.valid || !_javaScriptExecutor) { + if (!_valid || !_javaScriptExecutor) { return; } @@ -438,7 +457,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); sourceCodeModule.scriptData = sourceCode; [self enqueueApplicationScript:sourceCode url:self.bundleURL onComplete:^(NSError *loadError) { - if (!self.isValid) { + if (!_valid) { return; } @@ -490,7 +509,7 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void); { RCTAssertMainThread(); - if (!self.isValid || !self.loading) { + if (!_valid || !_loading) { return; } @@ -570,7 +589,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR - (void)invalidate { - if (!self.valid) { + if (!_valid) { return; } @@ -615,7 +634,6 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR _moduleDataByName = nil; _moduleDataByID = nil; _moduleClassesByID = nil; - _modulesByName_DEPRECATED = nil; _frameUpdateObservers = nil; _pendingCalls = nil; @@ -771,7 +789,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR RCTFatal(error); } - if (!self.isValid) { + if (!_valid) { return; } [self handleBuffer:json batchEnded:YES]; @@ -793,7 +811,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR RCTFatal(error); } - if (!self.isValid) { + if (!_valid) { return; } [self handleBuffer:json batchEnded:YES]; @@ -931,7 +949,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR methodID:(NSUInteger)methodID params:(NSArray *)params { - if (!self.isValid) { + if (!_valid) { return NO; } @@ -989,7 +1007,6 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR [self updateJSDisplayLinkState]; - RCTProfileImmediateEvent(0, @"JS Thread Tick", displayLink.timestamp, 'g'); RCT_PROFILE_END_EVENT(0, @"objc_call", nil); @@ -1022,24 +1039,3 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR } @end - -@implementation RCTBatchedBridge(Deprecated) - -- (NSDictionary *)modules -{ - if (!_modulesByName_DEPRECATED) { - // Check classes are set up - [self moduleClasses]; - NSMutableDictionary *modulesByName = [NSMutableDictionary new]; - for (NSString *moduleName in _moduleDataByName) { - id module = [self moduleForName:moduleName]; - if (module) { - modulesByName[moduleName] = module; - } - }; - _modulesByName_DEPRECATED = [modulesByName copy]; - } - return _modulesByName_DEPRECATED; -} - -@end diff --git a/React/Base/RCTBridge+Private.h b/React/Base/RCTBridge+Private.h index c954561ef..19f33c1a3 100644 --- a/React/Base/RCTBridge+Private.h +++ b/React/Base/RCTBridge+Private.h @@ -59,6 +59,11 @@ */ @property (nonatomic, weak, readonly) id javaScriptExecutor; +/** + * Used by RCTModuleData + */ +@property (nonatomic, assign, readonly) BOOL moduleSetupComplete; + /** * Used by RCTModuleData to register the module for frame updates after it is * lazily initialized. diff --git a/React/Base/RCTBridge.h b/React/Base/RCTBridge.h index 0b9fc6960..1d8548607 100644 --- a/React/Base/RCTBridge.h +++ b/React/Base/RCTBridge.h @@ -110,11 +110,18 @@ RCT_EXTERN BOOL RCTBridgeModuleClassIsRegistered(Class); * Retrieve a bridge module instance by name or class. Note that modules are * lazily instantiated, so calling these methods for the first time with a given * module name/class may cause the class to be sychronously instantiated, - * blocking both the calling thread and main thread for a short time. + * potentially blocking both the calling thread and main thread for a short time. */ - (id)moduleForName:(NSString *)moduleName; - (id)moduleForClass:(Class)moduleClass; +/** + * Convenience method for retrieving all modules conforming to a given protocol. + * Modules will be sychronously instantiated if they haven't already been, + * potentially blocking both the calling thread and main thread for a short time. + */ +- (NSArray *)modulesConformingToProtocol:(Protocol *)protocol; + /** * All registered bridge module classes. */ @@ -173,31 +180,3 @@ RCT_EXTERN BOOL RCTBridgeModuleClassIsRegistered(Class); - (BOOL)isBatchActive; @end - -/** - * These features are deprecated and should not be used. - */ -@interface RCTBridge (Deprecated) - -/** - * This notification used to fire after all native modules has been initialized, - * but now that native modules are instantiated lazily on demand, its original - * purpose is meaningless. - * - * If you need to access a module, you can do so as soon as the bridge has been - * initialized, by calling `[bridge moduleForClass:]`. If you need to know when - * an individual module has been instantiated, add an observer for the - * `RCTDidInitializeModuleNotification` instead. - */ -RCT_EXTERN NSString *const RCTDidCreateNativeModules -__deprecated_msg("Use RCTDidInitializeModuleNotification to observe init of individual modules"); - -/** - * Accessing the modules property causes all modules to be eagerly initialized, - * which stalls the main thread. Use moduleClasses to enumerate through modules - * without causing them to be instantiated. - */ -@property (nonatomic, copy, readonly) NSDictionary *modules -__deprecated_msg("Use moduleClasses and/or moduleForName: instead"); - -@end diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index aa939eae6..cd3edd45f 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -104,34 +104,6 @@ dispatch_queue_t RCTJSThread; // Set up JS thread RCTJSThread = (id)kCFNull; - - if (RCT_DEBUG && !RCTRunningInTestEnvironment()) { - - // Set up module classes - static unsigned int classCount; - Class *classes = objc_copyClassList(&classCount); - - for (unsigned int i = 0; i < classCount; i++) - { - Class cls = classes[i]; - Class superclass = cls; - while (superclass) - { - if (class_conformsToProtocol(superclass, @protocol(RCTBridgeModule))) - { - if (![RCTModuleClasses containsObject:cls]) { - RCTLogWarn(@"Class %@ was not exported. Did you forget to use " - "RCT_EXPORT_MODULE()?", cls); - } - break; - } - superclass = class_getSuperclass(superclass); - } - } - - free(classes); - - } }); } @@ -238,6 +210,20 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init) return [self moduleForName:RCTBridgeModuleNameForClass(moduleClass)]; } +- (NSArray *)modulesConformingToProtocol:(Protocol *)protocol +{ + NSMutableArray *modules = [NSMutableArray new]; + for (Class moduleClass in self.moduleClasses) { + if ([moduleClass conformsToProtocol:protocol]) { + id module = [self moduleForClass:moduleClass]; + if (module) { + [modules addObject:module]; + } + } + } + return [modules copy]; +} + - (RCTEventDispatcher *)eventDispatcher { return [self moduleForClass:[RCTEventDispatcher class]]; @@ -314,14 +300,3 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init) } @end - -@implementation RCTBridge(Deprecated) - -NSString *const RCTDidCreateNativeModules = @"RCTDidCreateNativeModules"; - -- (NSDictionary *)modules -{ - return self.batchedBridge.modules; -} - -@end diff --git a/React/Base/RCTModuleData.h b/React/Base/RCTModuleData.h index 6d5b00c7a..a851b2a38 100644 --- a/React/Base/RCTModuleData.h +++ b/React/Base/RCTModuleData.h @@ -21,21 +21,7 @@ bridge:(RCTBridge *)bridge NS_DESIGNATED_INITIALIZER; - (instancetype)initWithModuleInstance:(id)instance - bridge:(RCTBridge *)bridge; - -/** - * Sets the bridge for the module instance. This is only needed when using the - * `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; + bridge:(RCTBridge *)bridge NS_DESIGNATED_INITIALIZER; /** * Calls `constantsToExport` on the module and stores the result. Note that @@ -59,6 +45,11 @@ */ @property (nonatomic, assign, readonly) BOOL hasInstance; +/** + * Returns YES if module instance must be created on the main thread. + */ +@property (nonatomic, assign, readonly) BOOL requiresMainThreadSetup; + /** * Returns the current module instance. Note that this will init the instance * if it has not already been created. To check if the module instance exists @@ -73,9 +64,8 @@ @property (nonatomic, strong, readonly) dispatch_queue_t methodQueue; /** - * Returns the module config. Note that this will init the module if it has - * not already been created. This method can be called on any thread, but will - * block the main thread briefly if the module implements `constantsToExport`. + * Returns the module config. Calls `gatherConstants` internally, so the same + * usage caveats apply. */ @property (nonatomic, copy, readonly) NSArray *config; diff --git a/React/Base/RCTModuleData.m b/React/Base/RCTModuleData.m index 93427f52e..b3569db11 100644 --- a/React/Base/RCTModuleData.m +++ b/React/Base/RCTModuleData.m @@ -28,17 +28,37 @@ @synthesize instance = _instance; @synthesize methodQueue = _methodQueue; +- (void)setUp +{ + _implementsBatchDidComplete = [_moduleClass instancesRespondToSelector:@selector(batchDidComplete)]; + _implementsPartialBatchDidFlush = [_moduleClass instancesRespondToSelector:@selector(partialBatchDidFlush)]; + + _instanceLock = [NSLock new]; + + static IMP objectInitMethod; + static SEL setBridgeSelector; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + objectInitMethod = [NSObject instanceMethodForSelector:@selector(init)]; + setBridgeSelector = NSSelectorFromString(@"setBridge:"); + }); + + // If a module overrides `init`, `setBridge:`, or `constantsToExport` then we + // must assume that it expects for both of those methods to be called on the + // main thread, because those methods often need to access UIKit. + _requiresMainThreadSetup = _instance || + [_moduleClass instancesRespondToSelector:setBridgeSelector] || + RCTClassOverridesInstanceMethod(_moduleClass, @selector(constantsToExport)) || + [_moduleClass instanceMethodForSelector:@selector(init)] != objectInitMethod; +} + - (instancetype)initWithModuleClass:(Class)moduleClass bridge:(RCTBridge *)bridge { if ((self = [super init])) { - _moduleClass = moduleClass; _bridge = bridge; - - _implementsBatchDidComplete = [_moduleClass instancesRespondToSelector:@selector(batchDidComplete)]; - _implementsPartialBatchDidFlush = [_moduleClass instancesRespondToSelector:@selector(partialBatchDidFlush)]; - - _instanceLock = [NSLock new]; + _moduleClass = moduleClass; + [self setUp]; } return self; } @@ -46,8 +66,11 @@ - (instancetype)initWithModuleInstance:(id)instance bridge:(RCTBridge *)bridge { - if ((self = [self initWithModuleClass:[instance class] bridge:bridge])) { + if ((self = [super init])) { + _bridge = bridge; _instance = instance; + _moduleClass = [instance class]; + [self setUp]; } return self; } @@ -56,9 +79,47 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); #pragma mark - private setup methods +- (void)setUpInstanceAndBridge +{ + [_instanceLock lock]; + if (!_setupComplete && _bridge.valid) { + if (!_instance) { + if (RCT_DEBUG && _requiresMainThreadSetup) { + RCTAssertMainThread(); + } + _instance = [_moduleClass new]; + if (!_instance) { + // Module init returned nil, probably because automatic instantatiation + // of the module is not supported, and it is supposed to be passed in to + // the bridge constructor. Mark setup complete to avoid doing more work. + _setupComplete = YES; + RCTLogWarn(@"The module %@ is returning nil from its constructor. You " + "may need to instantiate it yourself and pass it into the " + "bridge.", _moduleClass); + } + } + // Bridge must be set before methodQueue is set up, as methodQueue + // initialization requires it (View Managers get their queue by calling + // self.bridge.uiManager.methodQueue) + [self setBridgeForInstance]; + } + [_instanceLock unlock]; + + // This is called outside of the lock in order to prevent deadlock issues + // because the logic in `setUpMethodQueue` can cause `moduleData.instance` + // to be accessed re-entrantly. + [self setUpMethodQueue]; + + // This is called outside of the lock in order to prevent deadlock issues + // because the logic in `finishSetupForInstance` can cause + // `moduleData.instance` to be accessed re-entrantly. + if (_bridge.moduleSetupComplete) { + [self finishSetupForInstance]; + } +} + - (void)setBridgeForInstance { - RCTAssert(_instance, @"setBridgeForInstance called before %@ initialized", self.name); if ([_instance respondsToSelector:@selector(bridge)] && _instance.bridge != _bridge) { @try { [(id)_instance setValue:_bridge forKey:@"bridge"]; @@ -73,9 +134,8 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); - (void)finishSetupForInstance { - if (!_setupComplete) { + if (!_setupComplete && _instance) { _setupComplete = YES; - [self setUpMethodQueue]; [_bridge registerModuleForFrameUpdates:_instance withModuleData:self]; [[NSNotificationCenter defaultCenter] postNotificationName:RCTDidInitializeModuleNotification object:_bridge @@ -85,8 +145,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); - (void)setUpMethodQueue { - if (!_methodQueue) { - RCTAssert(_instance, @"setUpMethodQueue called before %@ initialized", self.name); + if (_instance && !_methodQueue) { BOOL implementsMethodQueue = [_instance respondsToSelector:@selector(methodQueue)]; if (implementsMethodQueue) { _methodQueue = _instance.methodQueue; @@ -122,19 +181,19 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); - (id)instance { - [_instanceLock lock]; if (!_setupComplete) { - if (!_instance) { - _instance = [_moduleClass new]; + if (_requiresMainThreadSetup) { + // The chances of deadlock here are low, because module init very rarely + // calls out to other threads, however we can't control when a module might + // get accessed by client code during bridge setup, and a very low risk of + // deadlock is better than a fairly high risk of an assertion being thrown. + RCTExecuteOnMainThread(^{ + [self setUpInstanceAndBridge]; + }, YES); + } else { + [self setUpInstanceAndBridge]; } - // Bridge must be set before methodQueue is set up, as methodQueue - // initialization requires it (View Managers get their queue by calling - // self.bridge.uiManager.methodQueue) - [self setBridgeForInstance]; } - [_instanceLock unlock]; - - [self finishSetupForInstance]; return _instance; } @@ -183,8 +242,8 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); { if (!_constantsToExport) { if (RCTClassOverridesInstanceMethod(_moduleClass, @selector(constantsToExport))) { - RCTAssert(_instance, @"constantsToExport called before instance created."); RCTExecuteOnMainThread(^{ + [self setUpInstanceAndBridge]; _constantsToExport = [_instance constantsToExport] ?: @{}; }, YES); } else { @@ -195,7 +254,9 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); - (NSArray *)config { + [self gatherConstants]; __block NSDictionary *constants = _constantsToExport; + _constantsToExport = nil; // Not needed anymore if (constants.count == 0 && self.methods.count == 0) { return (id)kCFNull; // Nothing to export @@ -229,7 +290,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); - (dispatch_queue_t)methodQueue { - [self instance]; + (void)[self instance]; return _methodQueue; } diff --git a/React/Views/RCTViewManager.m b/React/Views/RCTViewManager.m index 4bb2d7981..88368290a 100644 --- a/React/Views/RCTViewManager.m +++ b/React/Views/RCTViewManager.m @@ -52,6 +52,9 @@ RCT_EXPORT_MODULE() - (dispatch_queue_t)methodQueue { + RCTAssert(_bridge, @"Bridge not set"); + RCTAssert(_bridge.uiManager, @"UIManager not initialized"); + RCTAssert(_bridge.uiManager.methodQueue, @"UIManager.methodQueue not initialized"); return _bridge.uiManager.methodQueue; }