diff --git a/React/Base/RCTBatchedBridge.mm b/React/Base/RCTBatchedBridge.mm index 5c90acca522..445a3e0572f 100644 --- a/React/Base/RCTBatchedBridge.mm +++ b/React/Base/RCTBatchedBridge.mm @@ -927,6 +927,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 be29341e314..831d11dd8d3 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -125,13 +125,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; @@ -169,7 +168,7 @@ struct RCTInstanceCallback : public InstanceCallback { - (JSGlobalContextRef)jsContextRef { - return (JSGlobalContextRef)self->_reactInstance->getJavaScriptContext(); + return (JSGlobalContextRef)(self->_reactInstance ? self->_reactInstance->getJavaScriptContext() : nullptr); } - (instancetype)initWithParentBridge:(RCTBridge *)bridge @@ -204,7 +203,7 @@ struct RCTInstanceCallback : public InstanceCallback { return self; } -- (void)runJSRunLoop ++ (void)runRunLoop { @autoreleasepool { RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"-[RCTCxxBridge runJSRunLoop] setup", nil); @@ -267,8 +266,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; @@ -493,7 +492,7 @@ struct RCTInstanceCallback : public InstanceCallback { if (_reactInstance) { // 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]); @@ -816,6 +815,7 @@ struct RCTInstanceCallback : public InstanceCallback { } RCTFatal(error); + // RN will stop, but let the rest of the app keep going. return; } @@ -826,27 +826,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); @@ -913,63 +913,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 @@ -1098,7 +1103,6 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR */ RCTProfileBeginFlowEvent(); - [self _runAfterLoad:^{ RCTProfileEndFlowEvent(); @@ -1189,25 +1193,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 {