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

Fix deadlock in bridge 

Summary:There was a deadlock in the bridge if a native module tried to dispatch an event through EventDispatcher (that thread would hold the mTeardownLock and want the mEventStaging lock) at the same time the EventDispatcher callback was triggered and tried to dispatch a call through JS (that thread would hold the mEventStaging lock and want the mTeardownLock).

Now there are two locks (lol). In the scenario above, the native module would hold the mJSToJavaTeardownLock and want the mEventStaging lock, while the EventDispatcher callback would hold the mEventStaging lock and want the mJavaToJSTeardownLock.

Reviewed By: lexs

Differential Revision: D3011526

fb-gh-sync-id: c3ebd5c14a6370d73caebf6c99fcba18a86c6ac1
shipit-source-id: c3ebd5c14a6370d73caebf6c99fcba18a86c6ac1
This commit is contained in:
Andy Street 2016-03-04 05:54:47 -08:00 committed by Facebook Github Bot 6
parent 1caebf175a
commit e64987d780
1 changed files with 27 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -53,9 +53,15 @@ public class CatalystInstanceImpl implements CatalystInstance {
private final TraceListener mTraceListener;
private final JavaScriptModuleRegistry mJSModuleRegistry;
private final JSBundleLoader mJSBundleLoader;
private final Object mTeardownLock = new Object();
private @Nullable ExecutorToken mMainExecutorToken;
// These locks prevent additional calls from going JS<->Java after the bridge has been torn down.
// There are separate ones for each direction because a JS to Java call can trigger a Java to JS
// call: this would cause a deadlock with a traditional mutex (maybe we should be using a reader-
// writer lock but then we'd have to worry about starving the destroy call).
private final Object mJSToJavaCallsTeardownLock = new Object();
private final Object mJavaToJSCallsTeardownLock = new Object();
// Access from native modules thread
private final NativeModuleRegistry mJavaRegistry;
private final NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler;
@ -172,7 +178,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
int methodId,
NativeArray arguments,
String tracingName) {
synchronized (mTeardownLock) {
synchronized (mJavaToJSCallsTeardownLock) {
if (mDestroyed) {
FLog.w(ReactConstants.TAG, "Calling JS function after bridge has been destroyed.");
return;
@ -189,7 +195,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
@DoNotStrip
@Override
public void invokeCallback(ExecutorToken executorToken, int callbackID, NativeArray arguments) {
synchronized (mTeardownLock) {
synchronized (mJavaToJSCallsTeardownLock) {
if (mDestroyed) {
FLog.w(ReactConstants.TAG, "Invoking JS callback after bridge has been destroyed.");
return;
@ -210,7 +216,10 @@ public class CatalystInstanceImpl implements CatalystInstance {
public void destroy() {
UiThreadUtil.assertOnUiThread();
synchronized (mTeardownLock) {
// This ordering is important. A JS to Java call that triggers a Java to JS call will also
// acquire these locks in the same order
synchronized (mJSToJavaCallsTeardownLock) {
synchronized (mJavaToJSCallsTeardownLock) {
if (mDestroyed) {
return;
}
@ -223,6 +232,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
synchronouslyDisposeBridgeOnJSThread();
}
}
mReactQueueConfiguration.destroy();
@ -401,7 +411,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
public void call(ExecutorToken executorToken, int moduleId, int methodId, ReadableNativeArray parameters) {
mReactQueueConfiguration.getNativeModulesQueueThread().assertIsOnThread();
synchronized (mTeardownLock) {
synchronized (mJSToJavaCallsTeardownLock) {
// Suppress any callbacks if destroyed - will only lead to sadness.
if (mDestroyed) {
return;
@ -419,7 +429,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
// native modules could be in a bad state so we don't want to call anything on them. We
// still want to trigger the debug listener since it allows instrumentation tests to end and
// check their assertions without waiting for a timeout.
synchronized (mTeardownLock) {
synchronized (mJSToJavaCallsTeardownLock) {
if (!mDestroyed) {
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "onBatchComplete");
try {
@ -440,7 +450,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
// Since onCatalystInstanceDestroy happens on the UI thread, we don't want to also execute
// this callback on the native modules thread at the same time. Longer term, onCatalystInstanceDestroy
// should probably be executed on the native modules thread as well instead.
synchronized (mTeardownLock) {
synchronized (mJSToJavaCallsTeardownLock) {
if (!mDestroyed) {
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "onExecutorUnregistered");
try {