diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index 533c8721f..6975d1ea9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -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,18 +216,22 @@ public class CatalystInstanceImpl implements CatalystInstance { public void destroy() { UiThreadUtil.assertOnUiThread(); - synchronized (mTeardownLock) { - if (mDestroyed) { - return; + // 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; + } + + // TODO: tell all APIs to shut down + mDestroyed = true; + mJavaRegistry.notifyCatalystInstanceDestroy(); + + Systrace.unregisterListener(mTraceListener); + + synchronouslyDisposeBridgeOnJSThread(); } - - // TODO: tell all APIs to shut down - mDestroyed = true; - mJavaRegistry.notifyCatalystInstanceDestroy(); - - Systrace.unregisterListener(mTraceListener); - - 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 {