Implement async method dispatch

Summary: Now that all the plumbing is done, this diff finally implements async method dispatch on the NativeModule thread.

Reviewed By: mdvacca

Differential Revision: D17480605

fbshipit-source-id: 992aab99954c488a0327144d84a1668a2b158d04

ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp

@@ -219,7 +219,7 @@ std::vector<std::string> getMethodArgTypesFromSignature(
 // needs to be done again
 // TODO (axe) Reuse existing implementation as needed - the exist in
 // MethodInvoker.cpp
-std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
+JNIArgs JavaTurboModule::convertJSIArgsToJNIArgs(
     JNIEnv *env,
     jsi::Runtime &rt,
     std::string methodName,
@@ -237,8 +237,21 @@ std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
         methodName, count, expectedArgumentCount);
   }
 
-  auto jargs =
-      std::vector<jvalue>(valueKind == PromiseKind ? count + 1 : count);
+  JNIArgs jniArgs(valueKind == PromiseKind ? count + 1 : count);
+  auto &jargs = jniArgs.args_;
+  auto &globalRefs = jniArgs.globalRefs_;
+
+  auto makeGlobalIfNecessary =
+      [&globalRefs, env, valueKind](jobject obj) -> jobject {
+    if (valueKind == VoidKind) {
+      jobject globalObj = env->NewGlobalRef(obj);
+      globalRefs.push_back(globalObj);
+      env->DeleteLocalRef(obj);
+      return globalObj;
+    }
+
+    return obj;
+  };
 
   jclass booleanClass = nullptr;
   jclass doubleClass = nullptr;
@@ -295,8 +308,8 @@ std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
 
       jmethodID doubleConstructor =
           env->GetMethodID(doubleClass, "<init>", "(D)V");
-      jarg->l =
-          env->NewObject(doubleClass, doubleConstructor, arg->getNumber());
+      jarg->l = makeGlobalIfNecessary(
+          env->NewObject(doubleClass, doubleConstructor, arg->getNumber()));
       continue;
     }
 
@@ -312,8 +325,8 @@ std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
 
       jmethodID booleanConstructor =
           env->GetMethodID(booleanClass, "<init>", "(Z)V");
-      jarg->l =
-          env->NewObject(booleanClass, booleanConstructor, arg->getBool());
+      jarg->l = makeGlobalIfNecessary(
+          env->NewObject(booleanClass, booleanConstructor, arg->getBool()));
       continue;
     }
 
@@ -323,7 +336,8 @@ std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
             "string", argIndex, methodName, arg, &rt);
       }
 
-      jarg->l = env->NewStringUTF(arg->getString(rt).utf8(rt).c_str());
+      jarg->l = makeGlobalIfNecessary(
+          env->NewStringUTF(arg->getString(rt).utf8(rt).c_str()));
       continue;
     }
 
@@ -336,7 +350,7 @@ std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
       auto dynamicFromValue = jsi::dynamicFromValue(rt, *arg);
       auto jParams =
           ReadableNativeArray::newObjectCxxArgs(std::move(dynamicFromValue));
-      jarg->l = jParams.release();
+      jarg->l = makeGlobalIfNecessary(jParams.release());
       continue;
     }
 
@@ -347,7 +361,8 @@ std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
       }
 
       jsi::Function fn = arg->getObject(rt).getFunction(rt);
-      jarg->l = createJavaCallbackFromJSIFunction(fn, rt, jsInvoker).release();
+      jarg->l = makeGlobalIfNecessary(
+          createJavaCallbackFromJSIFunction(fn, rt, jsInvoker).release());
       continue;
     }
 
@@ -360,12 +375,12 @@ std::vector<jvalue> JavaTurboModule::convertJSIArgsToJNIArgs(
       auto dynamicFromValue = jsi::dynamicFromValue(rt, *arg);
       auto jParams =
           ReadableNativeMap::createWithContents(std::move(dynamicFromValue));
-      jarg->l = jParams.release();
+      jarg->l = makeGlobalIfNecessary(jParams.release());
       continue;
     }
   }
 
-  return jargs;
+  return jniArgs;
 }
 
 jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) {
@@ -404,7 +419,15 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
    * object. For normal returns, we just create the return object.
    */
   unsigned int maxReturnObjects = 3;
-  unsigned int estimatedLocalRefCount = argCount + maxReturnObjects + buffer;
+
+  /**
+   * When the return type is void, all JNI LocalReferences are converted to
+   * GlobalReferences. The LocalReferences are then promptly deleted
+   * after the conversion.
+   */
+  unsigned int actualArgCount = valueKind == VoidKind ? 0 : argCount;
+  unsigned int estimatedLocalRefCount =
+      actualArgCount + maxReturnObjects + buffer;
 
   /**
    * This will push a new JNI stack frame for the LocalReferences in this
@@ -438,7 +461,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
   std::vector<std::string> methodArgTypes =
       getMethodArgTypesFromSignature(methodSignature);
 
-  std::vector<jvalue> jargs = convertJSIArgsToJNIArgs(
+  JNIArgs jniArgs = convertJSIArgsToJNIArgs(
       env,
       runtime,
       methodName,
@@ -448,10 +471,28 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
       jsInvoker_,
       valueKind);
 
+  auto &jargs = jniArgs.args_;
+  auto &globalRefs = jniArgs.globalRefs_;
+
   switch (valueKind) {
     case VoidKind: {
-      env->CallVoidMethodA(instance, methodID, jargs.data());
-      FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
+      nativeInvoker_->invokeAsync(
+          [jargs, globalRefs, methodID, instance_ = instance_]() mutable
+          -> void {
+            /**
+             * TODO(ramanpreet): Why do we have to require the environment
+             * again? Why does JNI crash when we use the env from the upper
+             * scope?
+             */
+            JNIEnv *env = jni::Environment::current();
+
+            env->CallVoidMethodA(instance_.get(), methodID, jargs.data());
+            FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
+
+            for (auto globalRef : globalRefs) {
+              env->DeleteGlobalRef(globalRef);
+            }
+          });
 
       return jsi::Value::undefined();
     }

ReactCommon/turbomodule/core/platform/android/JavaTurboModule.h

@@ -19,6 +19,12 @@
 namespace facebook {
 namespace react {
 
+struct JNIArgs {
+  JNIArgs(size_t count) : args_(count) {}
+  std::vector<jvalue> args_;
+  std::vector<jobject> globalRefs_;
+};
+
 struct JTurboModule : jni::JavaClass<JTurboModule> {
   static auto constexpr kJavaDescriptor =
       "Lcom/facebook/react/turbomodule/core/interfaces/TurboModule;";
@@ -59,7 +65,7 @@ class JSI_EXPORT JavaTurboModule : public TurboModule {
       jsi::Function &function,
       jsi::Runtime &rt,
       std::shared_ptr<CallInvoker> jsInvoker);
-  std::vector<jvalue> convertJSIArgsToJNIArgs(
+  JNIArgs convertJSIArgsToJNIArgs(
       JNIEnv *env,
       jsi::Runtime &rt,
       std::string methodName,