Skip to content

Commit

Permalink
timing fix for RCTCxxBridge.executeApplicationScript (#25991)
Browse files Browse the repository at this point in the history
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<NSDataBigString>(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: #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
  • Loading branch information
ahimberg authored and facebook-github-bot committed Mar 11, 2020
1 parent a3aaa47 commit 0c2db32
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions React/CxxBridge/RCTCxxBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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<JSIndexedRAMBundle>(sourceUrlStr.UTF8String);
std::unique_ptr<const JSBigString> 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<NSDataBigString>(script), sourceUrlStr.UTF8String, !async);
} else {
std::string methodName = async ? "loadApplicationScript" : "loadApplicationScriptSync";
Expand Down

0 comments on commit 0c2db32

Please sign in to comment.