diff --git a/React/Base/RCTBatchedBridge.mm b/React/Base/RCTBatchedBridge.mm index 2f5a6da2c2a..d56d61025ae 100644 --- a/React/Base/RCTBatchedBridge.mm +++ b/React/Base/RCTBatchedBridge.mm @@ -928,6 +928,10 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR { RCTAssertJSThread(); + if (!self.valid) { + return; + } + if (buffer != nil && buffer != (id)kCFNull) { _wasBatchActive = YES; [self handleBuffer:buffer]; diff --git a/React/Base/RCTModuleData.mm b/React/Base/RCTModuleData.mm index c1645549b38..340e9a5a7db 100644 --- a/React/Base/RCTModuleData.mm +++ b/React/Base/RCTModuleData.mm @@ -373,6 +373,8 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init); - (dispatch_queue_t)methodQueue { (void)[self instance]; + RCTAssert(_methodQueue != nullptr, @"Module %@ has no methodQueue (instance: %@, bridge.valid: %d)", + self, _instance, _bridge.valid); return _methodQueue; } diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index cd6844bdb68..bbf6b053480 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -149,13 +149,12 @@ struct RCTInstanceCallback : public InstanceCallback { [bridge_ partialBatchDidFlush]; [bridge_ batchDidComplete]; } - void incrementPendingJSCalls() override {} - void decrementPendingJSCalls() override {} }; @implementation RCTCxxBridge { BOOL _wasBatchActive; + BOOL _didInvalidate; NSMutableArray *_pendingCalls; std::atomic _pendingCount; @@ -194,7 +193,7 @@ struct RCTInstanceCallback : public InstanceCallback { - (JSGlobalContextRef)jsContextRef { - return (JSGlobalContextRef)self->_reactInstance->getJavaScriptContext(); + return (JSGlobalContextRef)(self->_reactInstance ? self->_reactInstance->getJavaScriptContext() : nullptr); } - (instancetype)initWithParentBridge:(RCTBridge *)bridge @@ -229,7 +228,7 @@ struct RCTInstanceCallback : public InstanceCallback { return self; } -- (void)runJSRunLoop ++ (void)runRunLoop { @autoreleasepool { RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"-[RCTCxxBridge runJSRunLoop] setup", nil); @@ -292,8 +291,8 @@ struct RCTInstanceCallback : public InstanceCallback { object:_parentBridge userInfo:@{@"bridge": self}]; // Set up the JS thread early - _jsThread = [[NSThread alloc] initWithTarget:self - selector:@selector(runJSRunLoop) + _jsThread = [[NSThread alloc] initWithTarget:[self class] + selector:@selector(runRunLoop) object:nil]; _jsThread.name = RCTJSThreadName; _jsThread.qualityOfService = NSOperationQualityOfServiceUserInteractive; @@ -522,7 +521,7 @@ struct RCTInstanceCallback : public InstanceCallback { // This is async, but any calls into JS are blocked by the m_syncReady CV in Instance _reactInstance->initializeBridge( - std::unique_ptr(new RCTInstanceCallback(self)), + std::make_unique(self), executorFactory, _jsMessageThread, [self _buildModuleRegistry]); @@ -845,6 +844,7 @@ struct RCTInstanceCallback : public InstanceCallback { } RCTFatal(error); + // RN will stop, but let the rest of the app keep going. return; } @@ -855,27 +855,27 @@ struct RCTInstanceCallback : public InstanceCallback { // Hack: once the bridge is invalidated below, it won't initialize any new native // modules. Initialize the redbox module now so we can still report this error. - [self redBox]; + RCTRedBox *redBox = [self redBox]; _loading = NO; _valid = NO; dispatch_async(dispatch_get_main_queue(), ^{ if (self->_jsMessageThread) { - auto thread = self->_jsMessageThread; - self->_jsMessageThread->runOnQueue([thread] { - thread->quitSynchronous(); - }); - self->_jsMessageThread.reset(); + // Make sure initializeBridge completed + self->_jsMessageThread->runOnQueueSync([] {}); } + self->_reactInstance.reset(); + self->_jsMessageThread.reset(); + [[NSNotificationCenter defaultCenter] postNotificationName:RCTJavaScriptDidFailToLoadNotification object:self->_parentBridge userInfo:@{@"bridge": self, @"error": error}]; if ([error userInfo][RCTJSRawStackTraceKey]) { - [self.redBox showErrorMessage:[error localizedDescription] - withRawStack:[error userInfo][RCTJSRawStackTraceKey]]; + [redBox showErrorMessage:[error localizedDescription] + withRawStack:[error userInfo][RCTJSRawStackTraceKey]]; } RCTFatal(error); @@ -942,63 +942,68 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR - (void)invalidate { - if (!_valid) { + if (_didInvalidate) { return; } RCTAssertMainQueue(); - RCTAssert(_reactInstance != nil, @"Can't complete invalidation without a react instance"); + RCTLogInfo(@"Invalidating %@ (parent: %@, executor: %@)", self, _parentBridge, [self executorClass]); _loading = NO; _valid = NO; + _didInvalidate = YES; + if ([RCTBridge currentBridge] == self) { [RCTBridge setCurrentBridge:nil]; } - // Invalidate modules - dispatch_group_t group = dispatch_group_create(); - for (RCTModuleData *moduleData in _moduleDataByID) { - // Be careful when grabbing an instance here, we don't want to instantiate - // any modules just to invalidate them. - if (![moduleData hasInstance]) { - continue; + // Stop JS instance and message thread + [self ensureOnJavaScriptThread:^{ + [self->_displayLink invalidate]; + self->_displayLink = nil; + + if (RCTProfileIsProfiling()) { + RCTProfileUnhookModules(self); } - if ([moduleData.instance respondsToSelector:@selector(invalidate)]) { - dispatch_group_enter(group); - [self dispatchBlock:^{ - [(id)moduleData.instance invalidate]; - dispatch_group_leave(group); - } queue:moduleData.methodQueue]; + // Invalidate modules + // We're on the JS thread (which we'll be suspending soon), so no new calls will be made to native modules after + // this completes. We must ensure all previous calls were dispatched before deallocating the instance (and module + // wrappers) or we may have invalid pointers still in flight. + dispatch_group_t moduleInvalidation = dispatch_group_create(); + for (RCTModuleData *moduleData in self->_moduleDataByID) { + // Be careful when grabbing an instance here, we don't want to instantiate + // any modules just to invalidate them. + if (![moduleData hasInstance]) { + continue; + } + + if ([moduleData.instance respondsToSelector:@selector(invalidate)]) { + dispatch_group_enter(moduleInvalidation); + [self dispatchBlock:^{ + [(id)moduleData.instance invalidate]; + dispatch_group_leave(moduleInvalidation); + } queue:moduleData.methodQueue]; + } + [moduleData invalidate]; } - [moduleData invalidate]; - } - dispatch_group_notify(group, dispatch_get_main_queue(), ^{ - [self ensureOnJavaScriptThread:^{ - [self->_displayLink invalidate]; - self->_displayLink = nil; + if (dispatch_group_wait(moduleInvalidation, dispatch_time(DISPATCH_TIME_NOW, 10 * NSEC_PER_SEC))) { + RCTLogError(@"Timed out waiting for modules to be invalidated"); + } - self->_reactInstance.reset(); - if (self->_jsMessageThread) { - self->_jsMessageThread->quitSynchronous(); - self->_jsMessageThread.reset(); - } + self->_reactInstance.reset(); + self->_jsMessageThread.reset(); - if (RCTProfileIsProfiling()) { - RCTProfileUnhookModules(self); - } + self->_moduleDataByName = nil; + self->_moduleDataByID = nil; + self->_moduleClassesByID = nil; + self->_pendingCalls = nil; - self->_moduleDataByName = nil; - self->_moduleDataByID = nil; - self->_moduleClassesByID = nil; - self->_pendingCalls = nil; - - [self->_jsThread cancel]; - self->_jsThread = nil; - CFRunLoopStop(CFRunLoopGetCurrent()); - }]; - }); + [self->_jsThread cancel]; + self->_jsThread = nil; + CFRunLoopStop(CFRunLoopGetCurrent()); + }]; } - (void)logMessage:(NSString *)message level:(NSString *)level @@ -1127,7 +1132,6 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR */ RCTProfileBeginFlowEvent(); - [self _runAfterLoad:^{ RCTProfileEndFlowEvent(); @@ -1218,25 +1222,25 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR if (!_reactInstance) { if (error) { *error = RCTErrorWithMessage( - @"Attempt to call sync callFunctionOnModule: on uninitialized bridge"); + @"callFunctionOnModule was called on uninitialized bridge"); } return nil; } else if (self.executorClass) { if (error) { *error = RCTErrorWithMessage( - @"sync callFunctionOnModule: can only be used with JSC executor"); + @"callFunctionOnModule can only be used with JSC executor"); } return nil; } else if (!self.valid) { if (error) { *error = RCTErrorWithMessage( - @"sync callFunctionOnModule: bridge is no longer valid"); + @"Bridge is no longer valid"); } return nil; } else if (self.loading) { if (error) { *error = RCTErrorWithMessage( - @"sync callFunctionOnModule: bridge is still loading"); + @"Bridge is still loading"); } return nil; } diff --git a/React/CxxModule/DispatchMessageQueueThread.h b/React/CxxModule/DispatchMessageQueueThread.h index f9dd9d33137..2877f5800bf 100644 --- a/React/CxxModule/DispatchMessageQueueThread.h +++ b/React/CxxModule/DispatchMessageQueueThread.h @@ -24,7 +24,6 @@ public: void runOnQueue(std::function&& func) override { dispatch_queue_t queue = moduleData_.methodQueue; - RCTAssert(queue != nullptr, @"Module %@ provided invalid queue", moduleData_); dispatch_block_t block = [func=std::move(func)] { func(); }; RCTAssert(block != nullptr, @"Invalid block generated in call to %@", moduleData_); if (queue && block) { diff --git a/ReactCommon/cxxreact/Instance.h b/ReactCommon/cxxreact/Instance.h index cb8504542f9..5089830b48e 100644 --- a/ReactCommon/cxxreact/Instance.h +++ b/ReactCommon/cxxreact/Instance.h @@ -27,9 +27,9 @@ class ModuleRegistry; struct InstanceCallback { virtual ~InstanceCallback() {} - virtual void onBatchComplete() = 0; - virtual void incrementPendingJSCalls() = 0; - virtual void decrementPendingJSCalls() = 0; + virtual void onBatchComplete() {} + virtual void incrementPendingJSCalls() {} + virtual void decrementPendingJSCalls() {} }; class RN_EXPORT Instance {