From bd95b22844e9facd2c6f34c644f4b33fd116c2d7 Mon Sep 17 00:00:00 2001 From: Andy Street Date: Tue, 1 Mar 2016 07:57:11 -0800 Subject: [PATCH] WebWorkers: Add ExecutorToken to route native module calls to/from workers Summary:To support native modules in web workers, native modules need to have an notion of the JS executor/thread that called into them in order to respond via Callback or JS module call to the right executor on the right thread. ExecutorToken is an object that only serves as a token that can be used to identify an executor/message queue thread in the bridge. It doesn't expose any methods. Native modules Callback objects automatically have this ExecutionContext attached -- JSModule calls for modules that support workers will need to supply an appropriate ExecutorToken when retrieving the JSModule implementation from the ReactContext. Reviewed By: mhorowitz Differential Revision: D2965458 fb-gh-sync-id: 6e354d4df8536d40b12d02bd055f6d06b4ca595d shipit-source-id: 6e354d4df8536d40b12d02bd055f6d06b4ca595d --- .../facebook/react/bridge/BaseJavaModule.java | 59 ++++--- .../facebook/react/bridge/CallbackImpl.java | 6 +- .../react/bridge/CatalystInstance.java | 4 +- .../react/bridge/CatalystInstanceImpl.java | 33 ++-- .../facebook/react/bridge/ExecutorToken.java | 24 +++ .../bridge/JavaScriptModuleRegistry.java | 56 +++++-- .../facebook/react/bridge/NativeModule.java | 22 ++- .../react/bridge/NativeModuleRegistry.java | 6 +- .../facebook/react/bridge/ReactBridge.java | 5 +- .../facebook/react/bridge/ReactCallback.java | 2 +- .../facebook/react/bridge/ReactContext.java | 7 + ReactAndroid/src/main/jni/react/BUCK | 2 + ReactAndroid/src/main/jni/react/Bridge.cpp | 150 +++++++++++++++--- ReactAndroid/src/main/jni/react/Bridge.h | 68 +++++++- .../src/main/jni/react/ExecutorToken.h | 54 +++++++ .../src/main/jni/react/ExecutorTokenFactory.h | 25 +++ .../src/main/jni/react/JSCExecutor.cpp | 31 ++-- ReactAndroid/src/main/jni/react/JSCExecutor.h | 16 +- .../src/main/jni/react/jni/Android.mk | 1 + ReactAndroid/src/main/jni/react/jni/BUCK | 5 +- .../src/main/jni/react/jni/JExecutorToken.cpp | 20 +++ .../src/main/jni/react/jni/JExecutorToken.h | 59 +++++++ .../jni/react/jni/JExecutorTokenFactory.h | 24 +++ .../src/main/jni/react/jni/OnLoad.cpp | 41 +++-- .../src/main/jni/react/jni/ProxyExecutor.cpp | 4 +- .../react/bridge/BaseJavaModuleTest.java | 6 +- 26 files changed, 614 insertions(+), 116 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/bridge/ExecutorToken.java create mode 100644 ReactAndroid/src/main/jni/react/ExecutorToken.h create mode 100644 ReactAndroid/src/main/jni/react/ExecutorTokenFactory.h create mode 100644 ReactAndroid/src/main/jni/react/jni/JExecutorToken.cpp create mode 100644 ReactAndroid/src/main/jni/react/jni/JExecutorToken.h create mode 100644 ReactAndroid/src/main/jni/react/jni/JExecutorTokenFactory.h diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java index 82d347c8c..afa121acb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java @@ -57,14 +57,14 @@ public abstract class BaseJavaModule implements NativeModule { } public abstract @Nullable T extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex); + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex); } static final private ArgumentExtractor ARGUMENT_EXTRACTOR_BOOLEAN = new ArgumentExtractor() { @Override public Boolean extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { return jsArguments.getBoolean(atIndex); } }; @@ -73,7 +73,7 @@ public abstract class BaseJavaModule implements NativeModule { new ArgumentExtractor() { @Override public Double extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { return jsArguments.getDouble(atIndex); } }; @@ -82,7 +82,7 @@ public abstract class BaseJavaModule implements NativeModule { new ArgumentExtractor() { @Override public Float extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { return (float) jsArguments.getDouble(atIndex); } }; @@ -91,7 +91,7 @@ public abstract class BaseJavaModule implements NativeModule { new ArgumentExtractor() { @Override public Integer extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { return (int) jsArguments.getDouble(atIndex); } }; @@ -100,7 +100,7 @@ public abstract class BaseJavaModule implements NativeModule { new ArgumentExtractor() { @Override public String extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { return jsArguments.getString(atIndex); } }; @@ -109,7 +109,7 @@ public abstract class BaseJavaModule implements NativeModule { new ArgumentExtractor() { @Override public ReadableNativeArray extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { return jsArguments.getArray(atIndex); } }; @@ -118,7 +118,7 @@ public abstract class BaseJavaModule implements NativeModule { new ArgumentExtractor() { @Override public ReadableMap extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { return jsArguments.getMap(atIndex); } }; @@ -127,12 +127,12 @@ public abstract class BaseJavaModule implements NativeModule { new ArgumentExtractor() { @Override public @Nullable Callback extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { if (jsArguments.isNull(atIndex)) { return null; } else { int id = (int) jsArguments.getDouble(atIndex); - return new CallbackImpl(catalystInstance, id); + return new CallbackImpl(catalystInstance, executorToken, id); } } }; @@ -146,11 +146,11 @@ public abstract class BaseJavaModule implements NativeModule { @Override public Promise extractArgument( - CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) { + CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray jsArguments, int atIndex) { Callback resolve = ARGUMENT_EXTRACTOR_CALLBACK - .extractArgument(catalystInstance, jsArguments, atIndex); + .extractArgument(catalystInstance, executorToken, jsArguments, atIndex); Callback reject = ARGUMENT_EXTRACTOR_CALLBACK - .extractArgument(catalystInstance, jsArguments, atIndex + 1); + .extractArgument(catalystInstance, executorToken, jsArguments, atIndex + 1); return new PromiseImpl(resolve, reject); } }; @@ -174,9 +174,21 @@ public abstract class BaseJavaModule implements NativeModule { } private ArgumentExtractor[] buildArgumentExtractors(Class[] paramTypes) { - ArgumentExtractor[] argumentExtractors = new ArgumentExtractor[paramTypes.length]; - for (int i = 0; i < paramTypes.length; i += argumentExtractors[i].getJSArgumentsNeeded()) { - Class argumentClass = paramTypes[i]; + // Modules that support web workers are expected to take an ExecutorToken as the first + // parameter to all their @ReactMethod-annotated methods. We compensate for that here. + int executorTokenOffset = 0; + if (BaseJavaModule.this.supportsWebWorkers()) { + if (paramTypes[0] != ExecutorToken.class) { + throw new RuntimeException( + "Module " + BaseJavaModule.this + " supports web workers, but " + mMethod.getName() + + "does not take an ExecutorToken as its first parameter."); + } + executorTokenOffset = 1; + } + + ArgumentExtractor[] argumentExtractors = new ArgumentExtractor[paramTypes.length - executorTokenOffset]; + for (int i = 0; i < paramTypes.length - executorTokenOffset; i += argumentExtractors[i].getJSArgumentsNeeded()) { + Class argumentClass = paramTypes[i + executorTokenOffset]; if (argumentClass == Boolean.class || argumentClass == boolean.class) { argumentExtractors[i] = ARGUMENT_EXTRACTOR_BOOLEAN; } else if (argumentClass == Integer.class || argumentClass == int.class) { @@ -220,7 +232,7 @@ public abstract class BaseJavaModule implements NativeModule { } @Override - public void invoke(CatalystInstance catalystInstance, ReadableNativeArray parameters) { + public void invoke(CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray parameters) { Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "callJavaModuleMethod"); try { if (mJSArgumentsNeeded != parameters.size()) { @@ -229,11 +241,18 @@ public abstract class BaseJavaModule implements NativeModule { parameters.size() + " arguments, expected " + mJSArgumentsNeeded); } + // Modules that support web workers are expected to take an ExecutorToken as the first + // parameter to all their @ReactMethod-annotated methods. We compensate for that here. int i = 0, jsArgumentsConsumed = 0; + int executorTokenOffset = 0; + if (BaseJavaModule.this.supportsWebWorkers()) { + mArguments[0] = executorToken; + executorTokenOffset = 1; + } try { for (; i < mArgumentExtractors.length; i++) { - mArguments[i] = mArgumentExtractors[i].extractArgument( - catalystInstance, parameters, jsArgumentsConsumed); + mArguments[i + executorTokenOffset] = mArgumentExtractors[i].extractArgument( + catalystInstance, executorToken, parameters, jsArgumentsConsumed); jsArgumentsConsumed += mArgumentExtractors[i].getJSArgumentsNeeded(); } } catch (UnexpectedNativeTypeException e) { @@ -341,7 +360,7 @@ public abstract class BaseJavaModule implements NativeModule { public void onCatalystInstanceDestroy() { // do nothing } - + @Override public boolean supportsWebWorkers() { return false; diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CallbackImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CallbackImpl.java index 8b5153e5c..a7bc85802 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CallbackImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CallbackImpl.java @@ -15,15 +15,17 @@ package com.facebook.react.bridge; public final class CallbackImpl implements Callback { private final CatalystInstance mCatalystInstance; + private final ExecutorToken mExecutorToken; private final int mCallbackId; - public CallbackImpl(CatalystInstance bridge, int callbackId) { + public CallbackImpl(CatalystInstance bridge, ExecutorToken executorToken, int callbackId) { mCatalystInstance = bridge; + mExecutorToken = executorToken; mCallbackId = callbackId; } @Override public void invoke(Object... args) { - mCatalystInstance.invokeCallback(mCallbackId, Arguments.fromJavaArgs(args)); + mCatalystInstance.invokeCallback(mExecutorToken, mCallbackId, Arguments.fromJavaArgs(args)); } } 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 466f229d1..98ac8d763 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java @@ -14,7 +14,6 @@ import java.util.Collection; import com.facebook.react.bridge.queue.ReactQueueConfiguration; import com.facebook.react.common.annotations.VisibleForTesting; import com.facebook.proguard.annotations.DoNotStrip; -import com.facebook.react.common.annotations.VisibleForTesting; /** * A higher level API on top of the asynchronous JSC bridge. This provides an @@ -27,7 +26,7 @@ public interface CatalystInstance { // This is called from java code, so it won't be stripped anyway, but proguard will rename it, // which this prevents. @DoNotStrip - void invokeCallback(final int callbackID, final NativeArray arguments); + void invokeCallback(ExecutorToken executorToken, final int callbackID, final NativeArray arguments); /** * Destroys this catalyst instance, waiting for any other threads in ReactQueueConfiguration * (besides the UI thread) to finish running. Must be called from the UI thread so that we can @@ -45,6 +44,7 @@ public interface CatalystInstance { ReactQueueConfiguration getReactQueueConfiguration(); T getJSModule(Class jsInterface); + T getJSModule(ExecutorToken executorToken, Class jsInterface); T getNativeModule(Class nativeModuleInterface); Collection getNativeModules(); 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 40618ac95..df8b26604 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -54,6 +54,7 @@ public class CatalystInstanceImpl implements CatalystInstance { private final JavaScriptModuleRegistry mJSModuleRegistry; private final JSBundleLoader mJSBundleLoader; private final Object mTeardownLock = new Object(); + private @Nullable ExecutorToken mMainExecutorToken; // Access from native modules thread private final NativeModuleRegistry mJavaRegistry; @@ -113,6 +114,7 @@ public class CatalystInstanceImpl implements CatalystInstance { jsExecutor, new NativeModulesReactCallback(), mReactQueueConfiguration.getNativeModulesQueueThread()); + mMainExecutorToken = bridge.getMainExecutorToken(); } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } @@ -165,10 +167,11 @@ public class CatalystInstanceImpl implements CatalystInstance { } /* package */ void callFunction( - final int moduleId, - final int methodId, - final NativeArray arguments, - final String tracingName) { + ExecutorToken executorToken, + int moduleId, + int methodId, + NativeArray arguments, + String tracingName) { synchronized (mTeardownLock) { if (mDestroyed) { FLog.w(ReactConstants.TAG, "Calling JS function after bridge has been destroyed."); @@ -177,7 +180,7 @@ public class CatalystInstanceImpl implements CatalystInstance { incrementPendingJSCalls(); - Assertions.assertNotNull(mBridge).callFunction(moduleId, methodId, arguments, tracingName); + Assertions.assertNotNull(mBridge).callFunction(executorToken, moduleId, methodId, arguments, tracingName); } } @@ -185,7 +188,7 @@ public class CatalystInstanceImpl implements CatalystInstance { // which this prevents. @DoNotStrip @Override - public void invokeCallback(final int callbackID, final NativeArray arguments) { + public void invokeCallback(ExecutorToken executorToken, int callbackID, NativeArray arguments) { synchronized (mTeardownLock) { if (mDestroyed) { FLog.w(ReactConstants.TAG, "Invoking JS callback after bridge has been destroyed."); @@ -194,7 +197,7 @@ public class CatalystInstanceImpl implements CatalystInstance { incrementPendingJSCalls(); - Assertions.assertNotNull(mBridge).invokeCallback(callbackID, arguments); + Assertions.assertNotNull(mBridge).invokeCallback(executorToken, callbackID, arguments); } } @@ -269,7 +272,12 @@ public class CatalystInstanceImpl implements CatalystInstance { @Override public T getJSModule(Class jsInterface) { - return Assertions.assertNotNull(mJSModuleRegistry).getJavaScriptModule(jsInterface); + return getJSModule(Assertions.assertNotNull(mMainExecutorToken), jsInterface); + } + + @Override + public T getJSModule(ExecutorToken executorToken, Class jsInterface) { + return Assertions.assertNotNull(mJSModuleRegistry).getJavaScriptModule(executorToken, jsInterface); } @Override @@ -388,7 +396,7 @@ public class CatalystInstanceImpl implements CatalystInstance { private class NativeModulesReactCallback implements ReactCallback { @Override - public void call(int moduleId, int methodId, ReadableNativeArray parameters) { + public void call(ExecutorToken executorToken, int moduleId, int methodId, ReadableNativeArray parameters) { mReactQueueConfiguration.getNativeModulesQueueThread().assertIsOnThread(); // Suppress any callbacks if destroyed - will only lead to sadness. @@ -396,7 +404,7 @@ public class CatalystInstanceImpl implements CatalystInstance { return; } - mJavaRegistry.call(CatalystInstanceImpl.this, moduleId, methodId, parameters); + mJavaRegistry.call(CatalystInstanceImpl.this, executorToken, moduleId, methodId, parameters); } @Override @@ -441,12 +449,13 @@ public class CatalystInstanceImpl implements CatalystInstance { private class JSProfilerTraceListener implements TraceListener { @Override public void onTraceStarted() { - getJSModule(com.facebook.react.bridge.Systrace.class).setEnabled(true); + getJSModule(Assertions.assertNotNull(mMainExecutorToken), com.facebook.react.bridge.Systrace.class).setEnabled( + true); } @Override public void onTraceStopped() { - getJSModule(com.facebook.react.bridge.Systrace.class).setEnabled(false); + getJSModule(Assertions.assertNotNull(mMainExecutorToken), com.facebook.react.bridge.Systrace.class).setEnabled(false); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ExecutorToken.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ExecutorToken.java new file mode 100644 index 000000000..e23bbebfb --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ExecutorToken.java @@ -0,0 +1,24 @@ +package com.facebook.react.bridge; + +import com.facebook.jni.HybridData; +import com.facebook.proguard.annotations.DoNotStrip; + +/** + * Class corresponding to a JS VM that can call into native modules. In Java, this should + * just be treated as a black box to be used to help the framework route native->JS calls back to + * the proper JS VM. See {@link ReactContext#getJSModule(ExecutorToken, Class)} and + * {@link BaseJavaModule#supportsWebWorkers()}. + * + * Note: If your application doesn't use web workers, it will only have a single ExecutorToken + * per instance of React Native. + */ +@DoNotStrip +public class ExecutorToken { + + private final HybridData mHybridData; + + @DoNotStrip + private ExecutorToken(HybridData hybridData) { + mHybridData = hybridData; + } +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java index 093770fe0..9a48e686b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java @@ -12,12 +12,16 @@ package com.facebook.react.bridge; import javax.annotation.Nullable; import java.lang.Class; +import java.lang.ref.WeakReference; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.HashMap; +import java.util.WeakHashMap; +import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; +import com.facebook.react.common.ReactConstants; /** * Class responsible for holding all the {@link JavaScriptModule}s registered to this @@ -27,45 +31,71 @@ import com.facebook.infer.annotation.Assertions; */ /*package*/ class JavaScriptModuleRegistry { - private final HashMap, JavaScriptModule> mModuleInstances; + private final CatalystInstanceImpl mCatalystInstance; + private final WeakHashMap, JavaScriptModule>> mModuleInstances; + private final HashMap, JavaScriptModuleRegistration> mModuleRegistrations; public JavaScriptModuleRegistry( CatalystInstanceImpl instance, JavaScriptModulesConfig config) { - mModuleInstances = new HashMap<>(); + mCatalystInstance = instance; + mModuleInstances = new WeakHashMap<>(); + mModuleRegistrations = new HashMap<>(); for (JavaScriptModuleRegistration registration : config.getModuleDefinitions()) { - Class moduleInterface = registration.getModuleInterface(); - JavaScriptModule interfaceProxy = (JavaScriptModule) Proxy.newProxyInstance( - moduleInterface.getClassLoader(), - new Class[]{moduleInterface}, - new JavaScriptModuleInvocationHandler(instance, registration)); - - mModuleInstances.put(moduleInterface, interfaceProxy); + mModuleRegistrations.put(registration.getModuleInterface(), registration); } } - public T getJavaScriptModule(Class moduleInterface) { - return (T) Assertions.assertNotNull( - mModuleInstances.get(moduleInterface), - "JS module " + moduleInterface.getSimpleName() + " hasn't been registered!"); + public synchronized T getJavaScriptModule(ExecutorToken executorToken, Class moduleInterface) { + HashMap, JavaScriptModule> instancesForContext = + mModuleInstances.get(executorToken); + if (instancesForContext == null) { + instancesForContext = new HashMap<>(); + mModuleInstances.put(executorToken, instancesForContext); + } + + JavaScriptModule module = instancesForContext.get(moduleInterface); + if (module != null) { + return (T) module; + } + + JavaScriptModuleRegistration registration = + Assertions.assertNotNull( + mModuleRegistrations.get(moduleInterface), + "JS module " + moduleInterface.getSimpleName() + " hasn't been registered!"); + JavaScriptModule interfaceProxy = (JavaScriptModule) Proxy.newProxyInstance( + moduleInterface.getClassLoader(), + new Class[]{moduleInterface}, + new JavaScriptModuleInvocationHandler(executorToken, mCatalystInstance, registration)); + instancesForContext.put(moduleInterface, interfaceProxy); + return (T) interfaceProxy; } private static class JavaScriptModuleInvocationHandler implements InvocationHandler { + private final WeakReference mExecutorToken; private final CatalystInstanceImpl mCatalystInstance; private final JavaScriptModuleRegistration mModuleRegistration; public JavaScriptModuleInvocationHandler( + ExecutorToken executorToken, CatalystInstanceImpl catalystInstance, JavaScriptModuleRegistration moduleRegistration) { + mExecutorToken = new WeakReference<>(executorToken); mCatalystInstance = catalystInstance; mModuleRegistration = moduleRegistration; } @Override public @Nullable Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + ExecutorToken executorToken = mExecutorToken.get(); + if (executorToken == null) { + FLog.w(ReactConstants.TAG, "Dropping JS call, ExecutorToken went away..."); + return null; + } String tracingName = mModuleRegistration.getTracingName(method); mCatalystInstance.callFunction( + executorToken, mModuleRegistration.getModuleId(), mModuleRegistration.getMethodId(method), Arguments.fromJavaArgs(args), diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java index 8996547f2..e954bc901 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java @@ -23,7 +23,7 @@ import com.fasterxml.jackson.core.JsonGenerator; */ public interface NativeModule { interface NativeMethod { - void invoke(CatalystInstance catalystInstance, ReadableNativeArray parameters); + void invoke(CatalystInstance catalystInstance, ExecutorToken executorToken, ReadableNativeArray parameters); String getType(); } @@ -74,7 +74,25 @@ public interface NativeModule { void onCatalystInstanceDestroy(); /** - * Whether this native module supports calls from web workers. Ignored for now. + * In order to support web workers, a module must be aware that it can be invoked from multiple + * different JS VMs. Supporting web workers means recognizing things like: + * + * 1) ids (e.g. timer ids, request ids, etc.) may only unique on a per-VM basis + * 2) the module needs to make sure to enqueue callbacks and JS module calls to the correct VM + * + * In order to facilitate this, modules that support web workers will have all their @ReactMethod- + * annotated methods passed a {@link ExecutorToken} as the first parameter before any arguments + * from JS. This ExecutorToken internally maps to a specific JS VM and can be used by the + * framework to route calls appropriately. In order to make JS module calls correctly, start using + * the version of {@link ReactContext#getJSModule(ExecutorToken, Class)} that takes an + * ExecutorToken. It will ensure that any calls you dispatch to the returned object will go to + * the right VM. For Callbacks, you don't have to do anything special -- the framework + * automatically tags them with the correct ExecutorToken when the are created. + * + * Note: even though calls can come from multiple JS VMs on multiple threads, calls to this module + * will still only occur on a single thread. + * + * @return whether this module supports web workers. */ boolean supportsWebWorkers(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java index c8a59e8f7..0d5cc0e65 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java @@ -48,6 +48,7 @@ public class NativeModuleRegistry { /* package */ void call( CatalystInstance catalystInstance, + ExecutorToken executorToken, int moduleId, int methodId, ReadableNativeArray parameters) { @@ -55,7 +56,7 @@ public class NativeModuleRegistry { if (definition == null) { throw new RuntimeException("Call to unknown module: " + moduleId); } - definition.call(catalystInstance, methodId, parameters); + definition.call(catalystInstance, executorToken, methodId, parameters); } /* package */ void writeModuleDescriptions(JsonGenerator jg) throws IOException { @@ -164,12 +165,13 @@ public class NativeModuleRegistry { public void call( CatalystInstance catalystInstance, + ExecutorToken executorToken, int methodId, ReadableNativeArray parameters) { MethodRegistration method = this.methods.get(methodId); Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, method.tracingName); try { - this.methods.get(methodId).method.invoke(catalystInstance, parameters); + this.methods.get(methodId).method.invoke(catalystInstance, executorToken, parameters); } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactBridge.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactBridge.java index 1e9fe6748..3c8128244 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactBridge.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactBridge.java @@ -79,12 +79,13 @@ public class ReactBridge extends Countable { */ public native void loadScriptFromAssets(AssetManager assetManager, String assetName); public native void loadScriptFromFile(@Nullable String fileName, @Nullable String sourceURL); - public native void callFunction(int moduleId, int methodId, NativeArray arguments, String tracingName); - public native void invokeCallback(int callbackID, NativeArray arguments); + public native void callFunction(ExecutorToken executorToken, int moduleId, int methodId, NativeArray arguments, String tracingName); + public native void invokeCallback(ExecutorToken executorToken, int callbackID, NativeArray arguments); public native void setGlobalVariable(String propertyName, String jsonEncodedArgument); public native boolean supportsProfiling(); public native void startProfiler(String title); public native void stopProfiler(String title, String filename); + public native ExecutorToken getMainExecutorToken(); private native void handleMemoryPressureModerate(); private native void handleMemoryPressureCritical(); public native void destroy(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactCallback.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactCallback.java index 7e4376c56..0399f27cf 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactCallback.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactCallback.java @@ -15,7 +15,7 @@ import com.facebook.proguard.annotations.DoNotStrip; public interface ReactCallback { @DoNotStrip - void call(int moduleId, int methodId, ReadableNativeArray parameters); + void call(ExecutorToken executorToken, int moduleId, int methodId, ReadableNativeArray parameters); @DoNotStrip void onBatchComplete(); 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 4ef692a8b..8e7880b59 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -96,6 +96,13 @@ public class ReactContext extends ContextWrapper { return mCatalystInstance.getJSModule(jsInterface); } + public T getJSModule(ExecutorToken executorToken, Class jsInterface) { + if (mCatalystInstance == null) { + throw new RuntimeException("Trying to invoke JS before CatalystInstance has been set!"); + } + return mCatalystInstance.getJSModule(executorToken, jsInterface); + } + /** * @return the instance of the specified module interface associated with this ReactContext. */ diff --git a/ReactAndroid/src/main/jni/react/BUCK b/ReactAndroid/src/main/jni/react/BUCK index f9ac1edb5..4c9a9d66b 100644 --- a/ReactAndroid/src/main/jni/react/BUCK +++ b/ReactAndroid/src/main/jni/react/BUCK @@ -67,6 +67,8 @@ react_library( exported_headers = [ 'AlignStack.h', 'Bridge.h', + 'ExecutorToken.h', + 'ExecutorTokenFactory.h', 'Executor.h', 'JSCExecutor.h', 'JSCHelpers.h', diff --git a/ReactAndroid/src/main/jni/react/Bridge.cpp b/ReactAndroid/src/main/jni/react/Bridge.cpp index ed758d1f0..84568b16f 100644 --- a/ReactAndroid/src/main/jni/react/Bridge.cpp +++ b/ReactAndroid/src/main/jni/react/Bridge.cpp @@ -7,17 +7,26 @@ using fbsystrace::FbSystraceSection; using fbsystrace::FbSystraceAsyncFlow; #endif +#include #include "Platform.h" namespace facebook { namespace react { -Bridge::Bridge(JSExecutorFactory* jsExecutorFactory, Callback callback) : - m_callback(std::move(callback)), - m_destroyed(std::make_shared(false)), - m_mainJSMessageQueueThread(MessageQueues::getCurrentMessageQueueThread()) { - m_mainExecutor = jsExecutorFactory->createJSExecutor(this); +Bridge::Bridge( + JSExecutorFactory* jsExecutorFactory, + std::unique_ptr executorTokenFactory, + Callback callback) : + m_callback(std::move(callback)), + m_destroyed(std::make_shared(false)), + m_executorTokenFactory(std::move(executorTokenFactory)) { + std::unique_ptr mainExecutor = jsExecutorFactory->createJSExecutor(this); + // cached to avoid locked map lookup in the common case + m_mainExecutor = mainExecutor.get(); + m_mainExecutorToken = folly::make_unique(registerExecutor( + std::move(mainExecutor), + MessageQueues::getCurrentMessageQueueThread())); } // This must be called on the same thread on which the constructor was called. @@ -37,6 +46,7 @@ void Bridge::loadApplicationUnbundle( } void Bridge::callFunction( + ExecutorToken executorToken, const double moduleId, const double methodId, const folly::dynamic& arguments, @@ -52,11 +62,15 @@ void Bridge::callFunction( tracingName.c_str(), systraceCookie); #endif + + auto executorMessageQueueThread = getMessageQueueThread(executorToken); + if (executorMessageQueueThread == nullptr) { + LOG(WARNING) << "Dropping JS call for executor that has been unregistered..."; + return; + } + std::shared_ptr isDestroyed = m_destroyed; - m_mainJSMessageQueueThread->runOnQueue([=] () { - if (*isDestroyed) { - return; - } + executorMessageQueueThread->runOnQueue([=] () { #ifdef WITH_FBSYSTRACE FbSystraceAsyncFlow::end( TRACE_TAG_REACT_CXX_BRIDGE, @@ -64,11 +78,25 @@ void Bridge::callFunction( systraceCookie); FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, tracingName.c_str()); #endif - m_mainExecutor->callFunction(moduleId, methodId, arguments); + + if (*isDestroyed) { + return; + } + + JSExecutor *executor = getExecutor(executorToken); + if (executor == nullptr) { + LOG(WARNING) << "Dropping JS call for executor that has been unregistered..."; + return; + } + + // This is safe because we are running on the executor's thread: it won't + // destruct until after it's been unregistered (which we check above) and + // that will happen on this thread + executor->callFunction(moduleId, methodId, arguments); }); } -void Bridge::invokeCallback(const double callbackId, const folly::dynamic& arguments) { +void Bridge::invokeCallback(ExecutorToken executorToken, const double callbackId, const folly::dynamic& arguments) { if (*m_destroyed) { return; } @@ -80,11 +108,15 @@ void Bridge::invokeCallback(const double callbackId, const folly::dynamic& argum "", systraceCookie); #endif + + auto executorMessageQueueThread = getMessageQueueThread(executorToken); + if (executorMessageQueueThread == nullptr) { + LOG(WARNING) << "Dropping JS call for executor that has been unregistered..."; + return; + } + std::shared_ptr isDestroyed = m_destroyed; - m_mainJSMessageQueueThread->runOnQueue([=] () { - if (*isDestroyed) { - return; - } + executorMessageQueueThread->runOnQueue([=] () { #ifdef WITH_FBSYSTRACE FbSystraceAsyncFlow::end( TRACE_TAG_REACT_CXX_BRIDGE, @@ -92,7 +124,21 @@ void Bridge::invokeCallback(const double callbackId, const folly::dynamic& argum systraceCookie); FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, "Bridge.invokeCallback"); #endif - m_mainExecutor->invokeCallback(callbackId, arguments); + + if (*isDestroyed) { + return; + } + + JSExecutor *executor = getExecutor(executorToken); + if (executor == nullptr) { + LOG(WARNING) << "Dropping JS call for executor that has been unregistered..."; + return; + } + + // This is safe because we are running on the executor's thread: it won't + // destruct until after it's been unregistered (which we check above) and + // that will happen on this thread + executor->invokeCallback(callbackId, arguments); }); } @@ -124,17 +170,83 @@ void Bridge::handleMemoryPressureCritical() { m_mainExecutor->handleMemoryPressureCritical(); } -void Bridge::callNativeModules(const std::string& callJSON, bool isEndOfBatch) { +void Bridge::callNativeModules(JSExecutor& executor, const std::string& callJSON, bool isEndOfBatch) { if (*m_destroyed) { return; } - m_callback(parseMethodCalls(callJSON), isEndOfBatch); + m_callback(getTokenForExecutor(executor), parseMethodCalls(callJSON), isEndOfBatch); +} + +ExecutorToken Bridge::getMainExecutorToken() const { + return *m_mainExecutorToken.get(); +} + +ExecutorToken Bridge::registerExecutor( + std::unique_ptr executor, + std::shared_ptr messageQueueThread) { + auto token = m_executorTokenFactory->createExecutorToken(); + + std::lock_guard registrationGuard(m_registrationMutex); + + CHECK(m_executorTokenMap.find(executor.get()) == m_executorTokenMap.end()) + << "Trying to register an already registered executor!"; + + m_executorTokenMap.emplace(executor.get(), token); + m_executorMap.emplace( + token, + folly::make_unique(std::move(executor), std::move(messageQueueThread))); + + return token; +} + +std::unique_ptr Bridge::unregisterExecutor(ExecutorToken executorToken) { + std::unique_ptr executor; + + { + std::lock_guard registrationGuard(m_registrationMutex); + + auto it = m_executorMap.find(executorToken); + CHECK(it != m_executorMap.end()) + << "Trying to unregister an executor that was never registered!"; + + executor = std::move(it->second->executor_); + m_executorMap.erase(it); + m_executorTokenMap.erase(executor.get()); + } + + // TODO: Notify native modules that ExecutorToken destroyed + + return executor; +} + +MessageQueueThread* Bridge::getMessageQueueThread(const ExecutorToken& executorToken) { + std::lock_guard registrationGuard(m_registrationMutex); + auto it = m_executorMap.find(executorToken); + if (it == m_executorMap.end()) { + return nullptr; + } + return it->second->messageQueueThread_.get(); +} + +JSExecutor* Bridge::getExecutor(const ExecutorToken& executorToken) { + std::lock_guard registrationGuard(m_registrationMutex); + auto it = m_executorMap.find(executorToken); + if (it == m_executorMap.end()) { + return nullptr; + } + return it->second->executor_.get(); +} + +ExecutorToken Bridge::getTokenForExecutor(JSExecutor& executor) { + std::lock_guard registrationGuard(m_registrationMutex); + return m_executorTokenMap.at(&executor); } void Bridge::destroy() { *m_destroyed = true; + std::unique_ptr mainExecutor = unregisterExecutor(*m_mainExecutorToken); m_mainExecutor->destroy(); - m_mainExecutor.reset(); + mainExecutor.reset(); } } } diff --git a/ReactAndroid/src/main/jni/react/Bridge.h b/ReactAndroid/src/main/jni/react/Bridge.h index 16edc62ac..05a0e6418 100644 --- a/ReactAndroid/src/main/jni/react/Bridge.h +++ b/ReactAndroid/src/main/jni/react/Bridge.h @@ -7,6 +7,8 @@ #include #include +#include "ExecutorToken.h" +#include "ExecutorTokenFactory.h" #include "Executor.h" #include "MessageQueueThread.h" #include "MethodCall.h" @@ -22,14 +24,30 @@ struct dynamic; namespace facebook { namespace react { +class Bridge; +class ExecutorRegistration { +public: + ExecutorRegistration( + std::unique_ptr executor, + std::shared_ptr executorMessageQueueThread) : + executor_(std::move(executor)), + messageQueueThread_(executorMessageQueueThread) {} + + std::unique_ptr executor_; + std::shared_ptr messageQueueThread_; +}; + class Bridge { public: - typedef std::function, bool isEndOfBatch)> Callback; + typedef std::function, bool isEndOfBatch)> Callback; /** * This must be called on the main JS thread. */ - Bridge(JSExecutorFactory* jsExecutorFactory, Callback callback); + Bridge( + JSExecutorFactory* jsExecutorFactory, + std::unique_ptr executorTokenFactory, + Callback callback); virtual ~Bridge(); /** @@ -37,6 +55,7 @@ public: * arguments in JS. */ void callFunction( + ExecutorToken executorToken, const double moduleId, const double methodId, const folly::dynamic& args, @@ -45,7 +64,7 @@ public: /** * Invokes a callback with the cbID, and optional additional arguments in JS. */ - void invokeCallback(const double callbackId, const folly::dynamic& args); + void invokeCallback(ExecutorToken executorToken, const double callbackId, const folly::dynamic& args); /** * Starts the JS application from an "bundle", i.e. a JavaScript file that @@ -71,9 +90,38 @@ public: void handleMemoryPressureCritical(); /** + * Invokes a set of native module calls on behalf of the given executor. + * * TODO: get rid of isEndOfBatch */ - void callNativeModules(const std::string& callJSON, bool isEndOfBatch); + void callNativeModules(JSExecutor& executor, const std::string& callJSON, bool isEndOfBatch); + + /** + * Returns the ExecutorToken corresponding to the main JSExecutor. + */ + ExecutorToken getMainExecutorToken() const; + + /** + * Registers the given JSExecutor which runs on the given MessageQueueThread + * with the Bridge. Part of this registration is transfering ownership of this + * JSExecutor to the Bridge for the duration of the registration. + * + * Returns a ExecutorToken which can be used to refer to this JSExecutor + * in the Bridge. + */ + ExecutorToken registerExecutor( + std::unique_ptr executor, + std::shared_ptr executorMessageQueueThread); + + /** + * Unregisters a JSExecutor that was previously registered with this Bridge + * using registerExecutor. Use the ExecutorToken returned from this + * registerExecutor call. This method will return ownership of the unregistered + * executor to the caller for it to retain or tear down. + * + * Returns ownership of the unregistered executor. + */ + std::unique_ptr unregisterExecutor(ExecutorToken executorToken); /** * Synchronously tears down the bridge and the main executor. @@ -85,11 +133,19 @@ private: // on the same thread. In that case, the callback will try to run the task on m_callback which // will have been destroyed within ~Bridge(), thus causing a SIGSEGV. std::shared_ptr m_destroyed; - std::unique_ptr m_mainExecutor; - std::shared_ptr m_mainJSMessageQueueThread; + JSExecutor* m_mainExecutor; + std::unique_ptr m_mainExecutorToken; + std::unique_ptr m_executorTokenFactory; + std::unordered_map m_executorTokenMap; + std::unordered_map> m_executorMap; + std::mutex m_registrationMutex; #ifdef WITH_FBSYSTRACE std::atomic_uint_least32_t m_systraceCookie = ATOMIC_VAR_INIT(); #endif + + MessageQueueThread* getMessageQueueThread(const ExecutorToken& executorToken); + JSExecutor* getExecutor(const ExecutorToken& executorToken); + inline ExecutorToken getTokenForExecutor(JSExecutor& executor); }; } } diff --git a/ReactAndroid/src/main/jni/react/ExecutorToken.h b/ReactAndroid/src/main/jni/react/ExecutorToken.h new file mode 100644 index 000000000..a630e90d3 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/ExecutorToken.h @@ -0,0 +1,54 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include "Executor.h" + +namespace facebook { +namespace react { + +/** + * This class exists so that we have a type for the shared_ptr on ExecutorToken + * that implements a virtual destructor. + */ +class PlatformExecutorToken { +public: + virtual ~PlatformExecutorToken() {} +}; + +/** + * Class corresponding to a JS VM that can call into native modules. This is + * passed to native modules to allow their JS module calls/callbacks to be + * routed back to the proper JS VM on the proper thread. + */ +class ExecutorToken { +public: + /** + * This should only be used by the implementation of the platform ExecutorToken. + * Do not use as a client of ExecutorToken. + */ + explicit ExecutorToken(std::shared_ptr platformToken) : + platformToken_(platformToken) {} + + std::shared_ptr getPlatformExecutorToken() const { + return platformToken_; + } + + bool operator==(const ExecutorToken& other) const { + return platformToken_.get() == other.platformToken_.get(); + } + +private: + std::shared_ptr platformToken_; +}; + +} } + +namespace std { + template<> + struct hash { + const size_t operator()(const facebook::react::ExecutorToken& token) const { + return (size_t) token.getPlatformExecutorToken().get(); + } + }; +} diff --git a/ReactAndroid/src/main/jni/react/ExecutorTokenFactory.h b/ReactAndroid/src/main/jni/react/ExecutorTokenFactory.h new file mode 100644 index 000000000..5d2d42179 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/ExecutorTokenFactory.h @@ -0,0 +1,25 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include "ExecutorToken.h" +#include "Executor.h" + +namespace facebook { +namespace react { + +/** + * Class that knows how to create the platform-specific implementation + * of ExecutorToken. + */ +class ExecutorTokenFactory { +public: + virtual ~ExecutorTokenFactory() {} + + /** + * Creates a new ExecutorToken. + */ + virtual ExecutorToken createExecutorToken() const = 0; +}; + +} } diff --git a/ReactAndroid/src/main/jni/react/JSCExecutor.cpp b/ReactAndroid/src/main/jni/react/JSCExecutor.cpp index 01ccc8188..04856494a 100644 --- a/ReactAndroid/src/main/jni/react/JSCExecutor.cpp +++ b/ReactAndroid/src/main/jni/react/JSCExecutor.cpp @@ -225,7 +225,7 @@ void JSCExecutor::flush() { } // TODO: Make this a first class function instead of evaling. #9317773 std::string calls = executeJSCallWithJSC(m_context, "flushedQueue", std::vector()); - m_bridge->callNativeModules(calls, true); + m_bridge->callNativeModules(*this, calls, true); } void JSCExecutor::callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) { @@ -236,7 +236,7 @@ void JSCExecutor::callFunction(const double moduleId, const double methodId, con std::move(arguments), }; std::string calls = executeJSCallWithJSC(m_context, "callFunctionReturnFlushedQueue", std::move(call)); - m_bridge->callNativeModules(calls, true); + m_bridge->callNativeModules(*this, calls, true); } void JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) { @@ -246,7 +246,7 @@ void JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic& std::move(arguments) }; std::string calls = executeJSCallWithJSC(m_context, "invokeCallbackAndReturnFlushedQueue", std::move(call)); - m_bridge->callNativeModules(calls, true); + m_bridge->callNativeModules(*this, calls, true); } void JSCExecutor::setGlobalVariable(const std::string& propName, const std::string& jsonValue) { @@ -304,7 +304,7 @@ void JSCExecutor::handleMemoryPressureCritical() { } void JSCExecutor::flushQueueImmediate(std::string queueJSON) { - m_bridge->callNativeModules(queueJSON, false); + m_bridge->callNativeModules(*this, queueJSON, false); } void JSCExecutor::loadModule(uint32_t moduleId) { @@ -332,13 +332,22 @@ int JSCExecutor::addWebWorker( Object workerObj = Value(m_context, workerRef).asObject(); workerObj.makeProtected(); - m_ownedWorkers.emplace(std::piecewise_construct, std::forward_as_tuple(workerId), std::forward_as_tuple(std::move(worker), std::move(workerObj))); + JSCExecutor *workerPtr = worker.get(); + std::shared_ptr sharedMessageQueueThread = worker->m_messageQueueThread; + ExecutorToken token = m_bridge->registerExecutor( + std::move(worker), + std::move(sharedMessageQueueThread)); + + m_ownedWorkers.emplace( + std::piecewise_construct, + std::forward_as_tuple(workerId), + std::forward_as_tuple(workerPtr, token, std::move(workerObj))); return workerId; } void JSCExecutor::postMessageToOwnedWebWorker(int workerId, JSValueRef message, JSValueRef *exn) { - auto worker = m_ownedWorkers.at(workerId).getExecutor(); + auto worker = m_ownedWorkers.at(workerId).executor; std::string msgString = Value(m_context, message).toJSONString(); std::shared_ptr isWorkerDestroyed = worker->m_isDestroyed; @@ -390,10 +399,14 @@ void JSCExecutor::receiveMessageFromOwner(const std::string& msgString) { } void JSCExecutor::terminateOwnedWebWorker(int workerId) { - auto worker = m_ownedWorkers.at(workerId).getExecutor(); - std::shared_ptr workerMQT = worker->m_messageQueueThread; - worker->destroy(); + auto& workerRegistration = m_ownedWorkers.at(workerId); + std::shared_ptr workerMQT = workerRegistration.executor->m_messageQueueThread; + ExecutorToken workerExecutorToken = workerRegistration.executorToken; m_ownedWorkers.erase(workerId); + + std::unique_ptr worker = m_bridge->unregisterExecutor(workerExecutorToken); + worker->destroy(); + worker.reset(); workerMQT->quitSynchronous(); } diff --git a/ReactAndroid/src/main/jni/react/JSCExecutor.h b/ReactAndroid/src/main/jni/react/JSCExecutor.h index a10907ccf..7f41170c8 100644 --- a/ReactAndroid/src/main/jni/react/JSCExecutor.h +++ b/ReactAndroid/src/main/jni/react/JSCExecutor.h @@ -8,6 +8,7 @@ #include #include +#include "ExecutorToken.h" #include "Executor.h" #include "JSCHelpers.h" #include "Value.h" @@ -31,17 +32,14 @@ private: class JSCExecutor; class WorkerRegistration : public noncopyable { public: - explicit WorkerRegistration(std::unique_ptr executor, Object jsObj) : - jsObj(std::move(jsObj)), - executor(std::move(executor)) {} - - JSCExecutor* getExecutor() { - return executor.get(); - } + explicit WorkerRegistration(JSCExecutor* executor, ExecutorToken executorToken, Object jsObj) : + executor(executor), + executorToken(executorToken), + jsObj(std::move(jsObj)) {} + JSCExecutor *executor; + ExecutorToken executorToken; Object jsObj; -private: - std::unique_ptr executor; }; class JSCExecutor : public JSExecutor { diff --git a/ReactAndroid/src/main/jni/react/jni/Android.mk b/ReactAndroid/src/main/jni/react/jni/Android.mk index 9d4d9b2b5..7d46f9ef8 100644 --- a/ReactAndroid/src/main/jni/react/jni/Android.mk +++ b/ReactAndroid/src/main/jni/react/jni/Android.mk @@ -5,6 +5,7 @@ include $(CLEAR_VARS) LOCAL_MODULE := reactnativejni LOCAL_SRC_FILES := \ + JExecutorToken.cpp \ JMessageQueueThread.cpp \ JSCPerfLogging.cpp \ JSLoader.cpp \ diff --git a/ReactAndroid/src/main/jni/react/jni/BUCK b/ReactAndroid/src/main/jni/react/jni/BUCK index 4d23d95ad..34c030641 100644 --- a/ReactAndroid/src/main/jni/react/jni/BUCK +++ b/ReactAndroid/src/main/jni/react/jni/BUCK @@ -35,6 +35,7 @@ jni_library( header_namespace = 'react/jni', supported_platforms_regex = SUPPORTED_PLATFORMS, srcs = [ + 'JExecutorToken.cpp', 'JMessageQueueThread.cpp', 'JSCPerfLogging.cpp', 'JSLoader.cpp', @@ -46,12 +47,14 @@ jni_library( ], headers = [ 'JSLoader.h', - 'ProxyExecutor.h', + 'JExecutorToken.h', + 'JExecutorTokenFactory.h', 'JMessageQueueThread.h', 'JNativeRunnable.h', 'JniJSModulesUnbundle.h', 'JSCPerfLogging.h', 'JSLogging.h', + 'ProxyExecutor.h', 'WebWorkers.h', ], exported_headers = [ diff --git a/ReactAndroid/src/main/jni/react/jni/JExecutorToken.cpp b/ReactAndroid/src/main/jni/react/jni/JExecutorToken.cpp new file mode 100644 index 000000000..97e80948b --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/JExecutorToken.cpp @@ -0,0 +1,20 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include "JExecutorToken.h" + +using namespace facebook::jni; + +namespace facebook { +namespace react { + +ExecutorToken JExecutorToken::getExecutorToken(alias_ref jobj) { + std::lock_guard guard(createTokenGuard_); + auto sharedOwner = owner_.lock(); + if (!sharedOwner) { + sharedOwner = std::shared_ptr(new JExecutorTokenHolder(jobj)); + owner_ = sharedOwner; + } + return ExecutorToken(sharedOwner); +} + +} } diff --git a/ReactAndroid/src/main/jni/react/jni/JExecutorToken.h b/ReactAndroid/src/main/jni/react/jni/JExecutorToken.h new file mode 100644 index 000000000..ac348f0ec --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/JExecutorToken.h @@ -0,0 +1,59 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include + +#include + +#include + +using namespace facebook::jni; + +namespace facebook { +namespace react { + +class JExecutorTokenHolder; +class JExecutorToken : public HybridClass { +public: + static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ExecutorToken;"; + + ExecutorToken getExecutorToken(alias_ref jobj); + +private: + friend HybridBase; + friend JExecutorTokenHolder; + + JExecutorToken() {} + + std::weak_ptr owner_; + std::mutex createTokenGuard_; +}; + +/** + * Wrapper class to hold references to both the c++ and Java parts of the + * ExecutorToken object. The goal is to allow a reference to a token from either + * c++ or Java to keep both the Java object and c++ hybrid part alive. For c++ + * references, we accomplish this by having JExecutorTokenHolder keep a reference + * to the Java object (which has a reference to the JExecutorToken hybrid part). + * For Java references, we allow the JExecutorTokenHolder to be deallocated if there + * are no references to it in c++ from a PlatformExecutorToken, but will dynamically + * create a new one in JExecutorToken.getExecutorToken if needed. + */ +class JExecutorTokenHolder : public PlatformExecutorToken, public noncopyable { +public: + explicit JExecutorTokenHolder(alias_ref jobj) : + jobj_(make_global(jobj)), + impl_(cthis(jobj)) { + } + + JExecutorToken::javaobject getJobj() { + return jobj_.get(); + } + +private: + global_ref jobj_; + JExecutorToken *impl_; +}; + +} } diff --git a/ReactAndroid/src/main/jni/react/jni/JExecutorTokenFactory.h b/ReactAndroid/src/main/jni/react/jni/JExecutorTokenFactory.h new file mode 100644 index 000000000..d7d923111 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/JExecutorTokenFactory.h @@ -0,0 +1,24 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include +#include + +#include "JExecutorToken.h" + +using namespace facebook::jni; + +namespace facebook { +namespace react { + +class JExecutorTokenFactory : public ExecutorTokenFactory { +public: + virtual ExecutorToken createExecutorToken() const override { + auto jExecutorToken = JExecutorToken::newObjectCxxArgs(); + auto jExecutorTokenNativePart = cthis(jExecutorToken); + return jExecutorTokenNativePart->getExecutorToken(jExecutorToken); + } +}; + +} } diff --git a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp index d684b5823..bb70f2f28 100644 --- a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp +++ b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp @@ -16,6 +16,8 @@ #include #include #include +#include "JExecutorToken.h" +#include "JExecutorTokenFactory.h" #include "JNativeRunnable.h" #include "JSLoader.h" #include "ReadableNativeArray.h" @@ -580,7 +582,7 @@ static void logMarker(const std::string& marker) { env->DeleteLocalRef(jmarker); } -static void makeJavaCall(JNIEnv* env, jobject callback, MethodCall&& call) { +static void makeJavaCall(JNIEnv* env, ExecutorToken executorToken, jobject callback, MethodCall&& call) { if (call.arguments.isNull()) { return; } @@ -592,14 +594,21 @@ static void makeJavaCall(JNIEnv* env, jobject callback, MethodCall&& call) { #endif auto newArray = ReadableNativeArray::newObjectCxxArgs(std::move(call.arguments)); - env->CallVoidMethod(callback, gCallbackMethod, call.moduleId, call.methodId, newArray.get()); + env->CallVoidMethod( + callback, + gCallbackMethod, + static_cast(executorToken.getPlatformExecutorToken().get())->getJobj(), + call.moduleId, + call.methodId, + newArray.get()); } static void signalBatchComplete(JNIEnv* env, jobject callback) { env->CallVoidMethod(callback, gOnBatchCompleteMethod); } -static void dispatchCallbacksToJava(const RefPtr& weakCallback, +static void dispatchCallbacksToJava(ExecutorToken executorToken, + const RefPtr& weakCallback, const RefPtr& weakCallbackQueueThread, std::vector&& calls, bool isEndOfBatch) { @@ -615,7 +624,7 @@ static void dispatchCallbacksToJava(const RefPtr& weakCallback, return; } - auto runnableFunction = std::bind([weakCallback, isEndOfBatch] (std::vector& calls) { + auto runnableFunction = std::bind([executorToken, weakCallback, isEndOfBatch] (std::vector& calls) { auto env = Environment::current(); if (env->ExceptionCheck()) { FBLOGW("Dropped calls because of pending exception"); @@ -624,7 +633,7 @@ static void dispatchCallbacksToJava(const RefPtr& weakCallback, ResolvedWeakReference callback(weakCallback); if (callback) { for (auto&& call : calls) { - makeJavaCall(env, callback, std::move(call)); + makeJavaCall(env, executorToken, callback, std::move(call)); if (env->ExceptionCheck()) { return; } @@ -643,11 +652,12 @@ static void create(JNIEnv* env, jobject obj, jobject executor, jobject callback, jobject callbackQueueThread) { auto weakCallback = createNew(callback); auto weakCallbackQueueThread = createNew(callbackQueueThread); - auto bridgeCallback = [weakCallback, weakCallbackQueueThread] (std::vector calls, bool isEndOfBatch) { - dispatchCallbacksToJava(weakCallback, weakCallbackQueueThread, std::move(calls), isEndOfBatch); + auto bridgeCallback = [weakCallback, weakCallbackQueueThread] (ExecutorToken executorToken, std::vector calls, bool isEndOfBatch) { + dispatchCallbacksToJava(executorToken, weakCallback, weakCallbackQueueThread, std::move(calls), isEndOfBatch); }; auto nativeExecutorFactory = extractRefPtr(env, executor); - auto bridge = createNew(nativeExecutorFactory.get(), bridgeCallback); + auto executorTokenFactory = folly::make_unique(); + auto bridge = createNew(nativeExecutorFactory.get(), std::move(executorTokenFactory), bridgeCallback); setCountableForJava(env, obj, std::move(bridge)); } @@ -735,12 +745,13 @@ static void loadScriptFromFile(JNIEnv* env, jobject obj, jstring fileName, jstri env->CallStaticVoidMethod(markerClass, gLogMarkerMethod, env->NewStringUTF("loadScriptFromFile_exec")); } -static void callFunction(JNIEnv* env, jobject obj, jint moduleId, jint methodId, +static void callFunction(JNIEnv* env, jobject obj, JExecutorToken::jhybridobject jExecutorToken, jint moduleId, jint methodId, NativeArray::jhybridobject args, jstring tracingName) { auto bridge = extractRefPtr(env, obj); auto arguments = cthis(wrap_alias(args)); try { bridge->callFunction( + cthis(wrap_alias(jExecutorToken))->getExecutorToken(wrap_alias(jExecutorToken)), (double) moduleId, (double) methodId, std::move(arguments->array), @@ -751,12 +762,13 @@ static void callFunction(JNIEnv* env, jobject obj, jint moduleId, jint methodId, } } -static void invokeCallback(JNIEnv* env, jobject obj, jint callbackId, +static void invokeCallback(JNIEnv* env, jobject obj, JExecutorToken::jhybridobject jExecutorToken, jint callbackId, NativeArray::jhybridobject args) { auto bridge = extractRefPtr(env, obj); auto arguments = cthis(wrap_alias(args)); try { bridge->invokeCallback( + cthis(wrap_alias(jExecutorToken))->getExecutorToken(wrap_alias(jExecutorToken)), (double) callbackId, std::move(arguments->array) ); @@ -775,6 +787,12 @@ static jlong getJavaScriptContext(JNIEnv *env, jobject obj) { return (uintptr_t) bridge->getJavaScriptContext(); } +static jobject getMainExecutorToken(JNIEnv* env, jobject obj) { + auto bridge = extractRefPtr(env, obj); + auto token = bridge->getMainExecutorToken(); + return static_cast(token.getPlatformExecutorToken().get())->getJobj(); +} + static jboolean supportsProfiling(JNIEnv* env, jobject obj) { auto bridge = extractRefPtr(env, obj); return bridge->supportsProfiling() ? JNI_TRUE : JNI_FALSE; @@ -948,7 +966,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { }); jclass callbackClass = env->FindClass("com/facebook/react/bridge/ReactCallback"); - bridge::gCallbackMethod = env->GetMethodID(callbackClass, "call", "(IILcom/facebook/react/bridge/ReadableNativeArray;)V"); + bridge::gCallbackMethod = env->GetMethodID(callbackClass, "call", "(Lcom/facebook/react/bridge/ExecutorToken;IILcom/facebook/react/bridge/ReadableNativeArray;)V"); bridge::gOnBatchCompleteMethod = env->GetMethodID(callbackClass, "onBatchComplete", "()V"); jclass markerClass = env->FindClass("com/facebook/react/bridge/ReactMarker"); @@ -964,6 +982,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { makeNativeMethod("callFunction", bridge::callFunction), makeNativeMethod("invokeCallback", bridge::invokeCallback), makeNativeMethod("setGlobalVariable", bridge::setGlobalVariable), + makeNativeMethod("getMainExecutorToken", "()Lcom/facebook/react/bridge/ExecutorToken;", bridge::getMainExecutorToken), makeNativeMethod("supportsProfiling", bridge::supportsProfiling), makeNativeMethod("startProfiler", bridge::startProfiler), makeNativeMethod("stopProfiler", bridge::stopProfiler), diff --git a/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp b/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp index c249f0ad3..07743bed8 100644 --- a/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp +++ b/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp @@ -65,7 +65,7 @@ void ProxyExecutor::callFunction(const double moduleId, const double methodId, c std::move(arguments), }; std::string result = executeJSCallWithProxy(m_executor.get(), "callFunctionReturnFlushedQueue", std::move(call)); - m_bridge->callNativeModules(result, true); + m_bridge->callNativeModules(*this, result, true); } void ProxyExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) { @@ -74,7 +74,7 @@ void ProxyExecutor::invokeCallback(const double callbackId, const folly::dynamic std::move(arguments) }; std::string result = executeJSCallWithProxy(m_executor.get(), "invokeCallbackAndReturnFlushedQueue", std::move(call)); - m_bridge->callNativeModules(result, true); + m_bridge->callNativeModules(*this, result, true); } void ProxyExecutor::setGlobalVariable(const std::string& propName, const std::string& jsonValue) { diff --git a/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.java index 8a681ccb5..dec87ebe3 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.java @@ -52,21 +52,21 @@ public class BaseJavaModuleTest { public void testCallMethodWithoutEnoughArgs() throws Exception { BaseJavaModule.NativeMethod regularMethod = mMethods.get("regularMethod"); Mockito.stub(mArguments.size()).toReturn(1); - regularMethod.invoke(null, mArguments); + regularMethod.invoke(null, null, mArguments); } @Test(expected = NativeArgumentsParseException.class) public void testCallAsyncMethodWithoutEnoughArgs() throws Exception { BaseJavaModule.NativeMethod asyncMethod = mMethods.get("asyncMethod"); Mockito.stub(mArguments.size()).toReturn(2); - asyncMethod.invoke(null, mArguments); + asyncMethod.invoke(null, null, mArguments); } @Test() public void testCallAsyncMethodWithEnoughArgs() throws Exception { BaseJavaModule.NativeMethod asyncMethod = mMethods.get("asyncMethod"); Mockito.stub(mArguments.size()).toReturn(3); - asyncMethod.invoke(null, mArguments); + asyncMethod.invoke(null, null, mArguments); } private static class MethodsModule extends BaseJavaModule {