Skip to content
Permalink
Browse files

Protect against JS module call race condition while initializing reac…

…t instance

Summary: Fixes a race condition where JS module functions could be called in between ##initializeWithInstance(catalystInstance);## and ##CatalystInstance#runJSBundle##, before the BatchedBridge in JS was set up. We now guarantee that all JS module methods that are called after `ReactContext#hasActiveCatalystInstance()` returns true will have the batched bridge created and ready to use.

Reviewed By: lexs

Differential Revision: D3258651

fb-gh-sync-id: 66e66533cc86d185e7c865376d6a5cdc6520d2d4
fbshipit-source-id: 66e66533cc86d185e7c865376d6a5cdc6520d2d4
  • Loading branch information...
astreet authored and Facebook Github Bot 0 committed May 5, 2016
1 parent a69b883 commit a1ba0918ab2f03504066790922cbec841799de09
@@ -10,6 +10,8 @@

import javax.annotation.Nullable;

import java.util.concurrent.Callable;

import android.app.Instrumentation;
import android.content.Context;
import android.support.test.InstrumentationRegistry;
@@ -124,9 +126,21 @@ public static ReactTestFactory getReactTestFactory() {

@Override
public CatalystInstance build() {
CatalystInstance instance = builder.build();
testCase.initializeWithInstance(instance);
instance.runJSBundle();
final CatalystInstance instance = builder.build();
try {
instance.getReactQueueConfiguration().getJSQueueThread().callOnQueue(
new Callable<Void>() {
@Override
public Void call() throws Exception {
testCase.initializeWithInstance(instance);
instance.runJSBundle();
return null;
}
}).get();

} catch (Exception e) {
throw new RuntimeException(e);
}
testCase.waitForBridgeAndUIIdle();
return instance;
}
@@ -18,6 +18,8 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;

import android.app.Activity;
import android.app.Application;
@@ -799,7 +801,7 @@ private ReactApplicationContext createReactContext(
NativeModuleRegistry.Builder nativeRegistryBuilder = new NativeModuleRegistry.Builder();
JavaScriptModulesConfig.Builder jsModulesBuilder = new JavaScriptModulesConfig.Builder();

ReactApplicationContext reactContext = new ReactApplicationContext(mApplicationContext);
final ReactApplicationContext reactContext = new ReactApplicationContext(mApplicationContext);
if (mUseDeveloperSupport) {
reactContext.setNativeModuleCallExceptionHandler(mDevSupportManager);
}
@@ -862,7 +864,7 @@ private ReactApplicationContext createReactContext(

ReactMarker.logMarker(CREATE_CATALYST_INSTANCE_START);
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "createCatalystInstance");
CatalystInstance catalystInstance;
final CatalystInstance catalystInstance;
try {
catalystInstance = catalystInstanceBuilder.build();
} finally {
@@ -874,16 +876,37 @@ private ReactApplicationContext createReactContext(
catalystInstance.addBridgeIdleDebugListener(mBridgeIdleDebugListener);
}

reactContext.initializeWithInstance(catalystInstance);

ReactMarker.logMarker(RUN_JS_BUNDLE_START);
// RUN_JS_BUNDLE_END is in JSCExecutor.cpp
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "runJSBundle");
try {
catalystInstance.runJSBundle();
} finally {
// This will actually finish when `JSCExecutor#loadApplicationScript()` finishes
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
catalystInstance.getReactQueueConfiguration().getJSQueueThread().callOnQueue(
new Callable<Void>() {
@Override
public Void call() {
// We want to ensure that any code that checks ReactContext#hasActiveCatalystInstance
// can be sure that it's safe to call a JS module function. As JS module function calls
// execute on the JS thread, and this Runnable runs on the JS thread, at this point we
// know that no JS module function calls will be executed until after this Runnable completes.
//
// This means it is now safe to say the instance is initialized.
//
// The reason we call this here instead of after this Runnable completes is so that we can
// reduce the amount of time until the React instance is able to start accepting JS calls,
// and so that any native module calls that result from runJSBundle can access JS modules.
reactContext.initializeWithInstance(catalystInstance);

ReactMarker.logMarker(RUN_JS_BUNDLE_START);
// RUN_JS_BUNDLE_END is in JSCExecutor.cpp
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "runJSBundle");
try {
catalystInstance.runJSBundle();
} finally {
// This will actually finish when `JSCExecutor#loadApplicationScript()` finishes
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
}
return null;
}
}).get();
} catch (InterruptedException | ExecutionException e) {
throw new RuntimeException(e);
}

return reactContext;
@@ -140,33 +140,24 @@ private ReactBridge initializeBridge(

@Override
public void runJSBundle() {
try {
mJSBundleHasLoaded = mReactQueueConfiguration.getJSQueueThread().callOnQueue(
new Callable<Boolean>() {
@Override
public Boolean call() throws Exception {
Assertions.assertCondition(!mJSBundleHasLoaded, "JS bundle was already loaded!");

incrementPendingJSCalls();
mReactQueueConfiguration.getJSQueueThread().assertIsOnThread();
Assertions.assertCondition(!mJSBundleHasLoaded, "JS bundle was already loaded!");

Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "loadJSScript");
try {
mJSBundleLoader.loadScript(mBridge);
incrementPendingJSCalls();

// This is registered after JS starts since it makes a JS call
Systrace.registerListener(mTraceListener);
} catch (JSExecutionException e) {
mNativeModuleCallExceptionHandler.handleException(e);
} finally {
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
}
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "loadJSScript");
try {
mJSBundleLoader.loadScript(mBridge);

return true;
}
}).get();
} catch (Exception t) {
throw new RuntimeException(t);
// This is registered after JS starts since it makes a JS call
Systrace.registerListener(mTraceListener);
} catch (JSExecutionException e) {
mNativeModuleCallExceptionHandler.handleException(e);
} finally {
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
}

mJSBundleHasLoaded = true;
}

@Override
@@ -31,6 +31,10 @@
*/
public class ReactContext extends ContextWrapper {

private static final String EARLY_JS_ACCESS_EXCEPTION_MESSAGE =
"Tried to access a JS module before the React instance was fully set up. Calls to " +
"ReactContext#getJSModule should be protected by ReactContext#hasActiveCatalystInstance().";

private final CopyOnWriteArraySet<LifecycleEventListener> mLifecycleEventListeners =
new CopyOnWriteArraySet<>();
private final CopyOnWriteArraySet<ActivityEventListener> mActivityEventListeners =
@@ -92,14 +96,14 @@ public Object getSystemService(String name) {
*/
public <T extends JavaScriptModule> T getJSModule(Class<T> jsInterface) {
if (mCatalystInstance == null) {
throw new RuntimeException("Trying to invoke JS before CatalystInstance has been set!");
throw new RuntimeException(EARLY_JS_ACCESS_EXCEPTION_MESSAGE);
}
return mCatalystInstance.getJSModule(jsInterface);
}

public <T extends JavaScriptModule> T getJSModule(ExecutorToken executorToken, Class<T> jsInterface) {
if (mCatalystInstance == null) {
throw new RuntimeException("Trying to invoke JS before CatalystInstance has been set!");
throw new RuntimeException(EARLY_JS_ACCESS_EXCEPTION_MESSAGE);
}
return mCatalystInstance.getJSModule(executorToken, jsInterface);
}

0 comments on commit a1ba091

Please sign in to comment.
You can’t perform that action at this time.