Permalink
Browse files

Add locking around CatalystInstance.getJavaScriptContext()

Reviewed By: kathryngray

Differential Revision: D5861693

fbshipit-source-id: 226ff15622d5e1a8ae3ad4c63f1434bd95c1fa21
  • Loading branch information...
cwdick authored and facebook-github-bot committed Sep 22, 2017
1 parent c1058b1 commit e9aab0d4521b9473f2b6142649cd33564332db71
@@ -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 @@ void callFunction(
/**
* Get the C pointer (as a long) to the JavaScriptCore context associated with this instance.
*
* <p>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();
}
@@ -91,6 +91,8 @@ public String toString() {
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 @@ private CatalystInstanceImpl(
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 void run() {
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 void removeBridgeIdleDebugListener(NotThreadSafeBridgeIdleDebugListener l
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();
@@ -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;
}
}
@@ -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 boolean startActivityForResult(Intent intent, int code, Bundle bundle) {
}
/**
* 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();
}
}
@@ -30,6 +30,7 @@
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 void run() {
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
}

0 comments on commit e9aab0d

Please sign in to comment.