status-im
/
react-native
mirror of https://github.com/status-im/react-native.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Protect against JS module call race condition while initializing react 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
This commit is contained in:
Andy Street 2016-05-05 05:51:45 -07:00 committed by Facebook Github Bot 0
parent a69b883537
commit a1ba0918ab
4 changed files with 72 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java
View File

@ -10,6 +10,8 @@ package com.facebook.react.testing;
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 class ReactTestHelper {
@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;
}

45
ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java
View File

@ -18,6 +18,8 @@ import java.util.Collection;
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 @@ import static com.facebook.react.bridge.ReactMarkerConstants.RUN_JS_BUNDLE_START
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 @@ import static com.facebook.react.bridge.ReactMarkerConstants.RUN_JS_BUNDLE_START
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 @@ import static com.facebook.react.bridge.ReactMarkerConstants.RUN_JS_BUNDLE_START
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;

39
ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
View File

@ -140,33 +140,24 @@ public class CatalystInstanceImpl implements CatalystInstance {
@Override
public void runJSBundle() {
mReactQueueConfiguration.getJSQueueThread().assertIsOnThread();
Assertions.assertCondition(!mJSBundleHasLoaded, "JS bundle was already loaded!");
incrementPendingJSCalls();
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "loadJSScript");
try {
mJSBundleHasLoaded = mReactQueueConfiguration.getJSQueueThread().callOnQueue(
new Callable<Boolean>() {
@Override
public Boolean call() throws Exception {
Assertions.assertCondition(!mJSBundleHasLoaded, "JS bundle was already loaded!");
mJSBundleLoader.loadScript(mBridge);
incrementPendingJSCalls();
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "loadJSScript");
try {
mJSBundleLoader.loadScript(mBridge);
// 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);
}
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

8
ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java
View File

@ -31,6 +31,10 @@ import com.facebook.react.bridge.queue.ReactQueueConfiguration;
*/
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 class ReactContext extends ContextWrapper {
*/
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);
}