From 0c2db3256f6cbb3ec564e0f183a52a439ed33f52 Mon Sep 17 00:00:00 2001 From: Andy Himberger Date: Tue, 10 Mar 2020 18:12:05 -0700 Subject: [PATCH] timing fix for RCTCxxBridge.executeApplicationScript (#25991) Summary: In one of our test apps (actually on Mac not iOS, but same code) we very consistently crash in RCTCxxBridge.executeApplicationScript when js debugging, due to a timing issue where another thread has reset _reactInstance in between the null check on self->_reactInstance and usage of it on these lines: ``` } else if (self->_reactInstance) { self->_reactInstance->loadScriptFromString(std::make_unique(script), ``` The thread doing the reset is doing so switching the executorClass to WebSocketExecutor. In the scenario we crash, the packager has a bundle ready and quickly returns it, though its a 34MB string being passed to NSDataBigString which must be taking long enough for the other thread to get a chance to reset. ## Changelog [iOS] [Fixed] - Fix crash in RCTCxxBridge.executeApplicationScript Pull Request resolved: https://github.com/facebook/react-native/pull/25991 Test Plan: Ran apple code path in normal from bundle file and js debugging scenarios. Reviewed By: shergin Differential Revision: D19186065 Pulled By: hramos fbshipit-source-id: ae1d4b5b50b7fb33b74aba21addc2978e917479f --- React/CxxBridge/RCTCxxBridge.mm | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index 31353856afe686..b4c97ed4667fe8 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -1334,19 +1334,23 @@ - (void)executeApplicationScript:(NSData *)script url:(NSURL *)url async:(BOOL)a [[NSNotificationCenter defaultCenter] postNotificationName:RCTJavaScriptWillStartExecutingNotification object:self->_parentBridge userInfo:@{@"bridge" : self}]; + + // hold a local reference to reactInstance in case a parallel thread + // resets it between null check and usage + auto reactInstance = self->_reactInstance; if (isRAMBundle(script)) { [self->_performanceLogger markStartForTag:RCTPLRAMBundleLoad]; auto ramBundle = std::make_unique(sourceUrlStr.UTF8String); std::unique_ptr scriptStr = ramBundle->getStartupCode(); [self->_performanceLogger markStopForTag:RCTPLRAMBundleLoad]; [self->_performanceLogger setValue:scriptStr->size() forTag:RCTPLRAMStartupCodeSize]; - if (self->_reactInstance) { + if (reactInstance) { auto registry = RAMBundleRegistry::multipleBundlesRegistry(std::move(ramBundle), JSIndexedRAMBundle::buildFactory()); - self->_reactInstance->loadRAMBundle(std::move(registry), std::move(scriptStr), sourceUrlStr.UTF8String, !async); + reactInstance->loadRAMBundle(std::move(registry), std::move(scriptStr), sourceUrlStr.UTF8String, !async); } - } else if (self->_reactInstance) { - self->_reactInstance->loadScriptFromString( + } else if (reactInstance) { + reactInstance->loadScriptFromString( std::make_unique(script), sourceUrlStr.UTF8String, !async); } else { std::string methodName = async ? "loadApplicationScript" : "loadApplicationScriptSync";