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 82d347c8c8f18f..afa121acb40232 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 int getJSArgumentsNeeded() { } 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 Boolean extractArgument( 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 Double extractArgument( 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 Float extractArgument( 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 Integer extractArgument( 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 String extractArgument( 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 ReadableNativeArray extractArgument( 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 ReadableMap extractArgument( 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 int getJSArgumentsNeeded() { @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 JavaMethod(Method method) { } 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 @@ private String getAffectedRange(int startIndex, int jsArgumentsNeeded) { } @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 void invoke(CatalystInstance catalystInstance, ReadableNativeArray parame 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 void onReactBridgeInitialized(ReactBridge bridge) { 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 8b5153e5cc51eb..a7bc858026eece 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 @@ 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 466f229d151d76..98ac8d76398b97 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 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 40618ac95c159c..df8b26604c47c8 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 @@ private ReactBridge initializeBridge( jsExecutor, new NativeModulesReactCallback(), mReactQueueConfiguration.getNativeModulesQueueThread()); + mMainExecutorToken = bridge.getMainExecutorToken(); } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } @@ -165,10 +167,11 @@ public Boolean call() throws Exception { } /* 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 Boolean call() throws Exception { incrementPendingJSCalls(); - Assertions.assertNotNull(mBridge).callFunction(moduleId, methodId, arguments, tracingName); + Assertions.assertNotNull(mBridge).callFunction(executorToken, moduleId, methodId, arguments, tracingName); } } @@ -185,7 +188,7 @@ public Boolean call() throws Exception { // 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 void invokeCallback(final int callbackID, final NativeArray arguments) { incrementPendingJSCalls(); - Assertions.assertNotNull(mBridge).invokeCallback(callbackID, arguments); + Assertions.assertNotNull(mBridge).invokeCallback(executorToken, callbackID, arguments); } } @@ -269,7 +272,12 @@ public ReactQueueConfiguration getReactQueueConfiguration() { @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 @@ private void decrementPendingJSCalls() { 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 void call(int moduleId, int methodId, ReadableNativeArray parameters) { return; } - mJavaRegistry.call(CatalystInstanceImpl.this, moduleId, methodId, parameters); + mJavaRegistry.call(CatalystInstanceImpl.this, executorToken, moduleId, methodId, parameters); } @Override @@ -441,12 +449,13 @@ public void run() { 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 00000000000000..e23bbebfb7d1df --- /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 093770fe05fd68..9a48e686bb550e 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 @@ 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 @@ */ /*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 8996547f257a0d..e954bc901552b5 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 @@ */ 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 @@ interface NativeMethod { 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 c8a59e8f73ac74..0d5cc0e65a78f5 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 @@ private NativeModuleRegistry( /* package */ void call( CatalystInstance catalystInstance, + ExecutorToken executorToken, int moduleId, int methodId, ReadableNativeArray parameters) { @@ -55,7 +56,7 @@ private 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 ModuleDefinition(int id, String name, NativeModule target) { 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 1e9fe67482db94..3c8128244f4b90 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 @@ private native void initialize( */ 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 7e4376c5685156..0399f27cf7a7cb 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 @@ 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 4ef692a8bdd18c..8e7880b594e259 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 T getJSModule(Class jsInterface) { 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 f9ac1edb5f8d48..4c9a9d66bf5fba 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 ed758d1f007983..84568b16ff1f56 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 16edc62ac10c92..05a0e64189c985 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 @@ class Bridge { * arguments in JS. */ void callFunction( + ExecutorToken executorToken, const double moduleId, const double methodId, const folly::dynamic& args, @@ -45,7 +64,7 @@ class Bridge { /** * 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 @@ class Bridge { 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 @@ class Bridge { // 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 00000000000000..a630e90d34a16c --- /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 00000000000000..5d2d42179073c9 --- /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 01ccc8188928c7..04856494a2ca11 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 a10907ccff0d4d..7f41170c8c44aa 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 @@ class JSCExecutorFactory : public JSExecutorFactory { 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 9d4d9b2b5fffad..7d46f9ef8bafb6 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 4d23d95ad3a094..34c030641a8fc7 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 00000000000000..97e80948b196a5 --- /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 00000000000000..ac348f0ecd731f --- /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 00000000000000..d7d923111395b0 --- /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 d684b5823c05bd..bb70f2f280ec70 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 c249f0ad31485c..07743bed896a57 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 8a681ccb51bfb4..dec87ebe3c6f0b 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 void setup() { 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 {