diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java index 9770f5202..518b9d32b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java @@ -9,13 +9,11 @@ package com.facebook.react.bridge; -import javax.annotation.Nullable; - -import java.util.Collection; - import com.facebook.proguard.annotations.DoNotStrip; import com.facebook.react.bridge.queue.ReactQueueConfiguration; import com.facebook.react.common.annotations.VisibleForTesting; +import java.util.Collection; +import javax.annotation.Nullable; /** * A higher level API on top of the asynchronous JSC bridge. This provides an @@ -93,6 +91,10 @@ public interface CatalystInstance /** * Get the C pointer (as a long) to the JavaScriptCore context associated with this instance. + * + *

Use the following pattern to ensure that the JS context is not cleared while you are using + * it: JavaScriptContextHolder jsContext = reactContext.getJavaScriptContextHolder() + * synchronized(jsContext) { nativeThingNeedingJsContext(jsContext.get()); } */ - long getJavaScriptContext(); + JavaScriptContextHolder getJavaScriptContextHolder(); } 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 cc254c83e..3f11e889c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -91,6 +91,8 @@ public class CatalystInstanceImpl implements CatalystInstance { private boolean mJSBundleHasLoaded; private @Nullable String mSourceURL; + private JavaScriptContextHolder mJavaScriptContextHolder; + // C++ parts private final HybridData mHybridData; private native static HybridData initHybrid(); @@ -126,6 +128,8 @@ public class CatalystInstanceImpl implements CatalystInstance { mNativeModuleRegistry.getJavaModules(this), mNativeModuleRegistry.getCxxModules()); Log.d(ReactConstants.TAG, "Initializing React Xplat Bridge after initializeBridge"); + + mJavaScriptContextHolder = new JavaScriptContextHolder(getJavaScriptContext()); } private static class BridgeCallback implements ReactCallback { @@ -333,6 +337,11 @@ public class CatalystInstanceImpl implements CatalystInstance { public void run() { // Kill non-UI threads from neutral third party // potentially expensive, so don't run on UI thread + + // contextHolder is used as a lock to guard against other users of the JS VM having + // the VM destroyed underneath them, so notify them before we resetNative + mJavaScriptContextHolder.clear(); + mHybridData.resetNative(); getReactQueueConfiguration().destroy(); Log.d(ReactConstants.TAG, "CatalystInstanceImpl.destroy() end"); @@ -436,7 +445,11 @@ public class CatalystInstanceImpl implements CatalystInstance { public native void setGlobalVariable(String propName, String jsonValue); @Override - public native long getJavaScriptContext(); + public JavaScriptContextHolder getJavaScriptContextHolder() { + return mJavaScriptContextHolder; + } + + private native long getJavaScriptContext(); private void incrementPendingJSCalls() { int oldPendingCalls = mPendingJSCalls.getAndIncrement(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptContextHolder.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptContextHolder.java new file mode 100644 index 000000000..edc3f2421 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptContextHolder.java @@ -0,0 +1,29 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +package com.facebook.react.bridge; + +import javax.annotation.concurrent.GuardedBy; + +/** + * Wrapper for JavaScriptContext native pointer. CatalystInstanceImpl creates this on demand, and + * will call clear() before destroying the VM. People who need the raw JavaScriptContext pointer + * can synchronize on this wrapper object to guarantee that it will not be destroyed. + */ +public class JavaScriptContextHolder { + @GuardedBy("this") + private long mContext; + + public JavaScriptContextHolder(long context) { + mContext = context; + } + + @GuardedBy("this") + public long get() { + return mContext; + } + + public synchronized void clear() { + mContext = 0; + } + +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index 80d2bab72..4148889d3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -9,22 +9,19 @@ package com.facebook.react.bridge; -import javax.annotation.Nullable; - -import java.lang.ref.WeakReference; -import java.util.concurrent.CopyOnWriteArraySet; - import android.app.Activity; import android.content.Context; import android.content.ContextWrapper; import android.content.Intent; import android.os.Bundle; import android.view.LayoutInflater; - import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.queue.MessageQueueThread; import com.facebook.react.bridge.queue.ReactQueueConfiguration; import com.facebook.react.common.LifecycleState; +import java.lang.ref.WeakReference; +import java.util.concurrent.CopyOnWriteArraySet; +import javax.annotation.Nullable; /** * Abstract ContextWrapper for Android application or activity {@link Context} and @@ -372,9 +369,12 @@ public class ReactContext extends ContextWrapper { } /** - * Get the C pointer (as a long) to the JavaScriptCore context associated with this instance. + * Get the C pointer (as a long) to the JavaScriptCore context associated with this instance. Use + * the following pattern to ensure that the JS context is not cleared while you are using it: + * JavaScriptContextHolder jsContext = reactContext.getJavaScriptContextHolder() + * synchronized(jsContext) { nativeThingNeedingJsContext(jsContext.get()); } */ - public long getJavaScriptContext() { - return mCatalystInstance.getJavaScriptContext(); + public JavaScriptContextHolder getJavaScriptContextHolder() { + return mCatalystInstance.getJavaScriptContextHolder(); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java index ecd3f9bab..df70ef88b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java @@ -30,6 +30,7 @@ import com.facebook.react.R; import com.facebook.react.bridge.CatalystInstance; import com.facebook.react.bridge.DefaultNativeModuleCallExceptionHandler; import com.facebook.react.bridge.JavaJSExecutor; +import com.facebook.react.bridge.JavaScriptContextHolder; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReactMarker; import com.facebook.react.bridge.ReactMarkerConstants; @@ -711,10 +712,12 @@ public class DevSupportManagerImpl implements return; } try { - long jsContext = mCurrentContext.getJavaScriptContext(); - Class clazz = Class.forName("com.facebook.react.packagerconnection.SamplingProfilerPackagerMethod"); - RequestHandler handler = (RequestHandler)clazz.getConstructor(long.class).newInstance(jsContext); - handler.onRequest(null, responder); + JavaScriptContextHolder jsContext = mCurrentContext.getJavaScriptContextHolder(); + synchronized (jsContext) { + Class clazz = Class.forName("com.facebook.react.packagerconnection.SamplingProfilerPackagerMethod"); + RequestHandler handler = (RequestHandler)clazz.getConstructor(long.class).newInstance(jsContext.get()); + handler.onRequest(null, responder); + } } catch (Exception e) { // Module not present }