Fix unsafe async call in CatalystInstance callback (facebook#40861)

Summary:
Pull Request resolved: facebook#40861

Reference `jobj_` from the async callback is unsafe, as the Java counterpart may have been deallocated by the time it's executed. Instead move the async call to Java.

Note that this method doesn't actually do anything in Fabric, it's used by the old renderer only.

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D50224974

fbshipit-source-id: c848b177753643febac1d6646d62a27ebace9238

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java

@@ -152,6 +152,7 @@ private CatalystInstanceImpl(
     mJavaScriptContextHolder = new JavaScriptContextHolder(getJavaScriptContext());
   }
 
+  @DoNotStrip
   private static class BridgeCallback implements ReactCallback {
     // We do this so the callback doesn't keep the CatalystInstanceImpl alive.
     // In this case, the callback is held in C++ code, so the GC can't see it
@@ -166,7 +167,10 @@ private static class BridgeCallback implements ReactCallback {
     public void onBatchComplete() {
       CatalystInstanceImpl impl = mOuter.get();
       if (impl != null) {
-        impl.mNativeModuleRegistry.onBatchComplete();
+        impl.mNativeModulesQueueThread.runOnQueue(
+            () -> {
+              impl.mNativeModuleRegistry.onBatchComplete();
+            });
       }
     }
 

packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp

@@ -7,12 +7,8 @@
 
 #include "CatalystInstanceImpl.h"
 
-#include <condition_variable>
 #include <fstream>
 #include <memory>
-#include <mutex>
-#include <sstream>
-#include <vector>
 
 #include <ReactCommon/CallInvokerHolder.h>
 #include <cxxreact/CxxNativeModule.h>
@@ -34,7 +30,6 @@
 
 #include <logger/react_native_log.h>
 
-#include "CxxModuleWrapper.h"
 #include "JReactCxxErrorHandler.h"
 #include "JReactSoftExceptionLogger.h"
 #include "JavaScriptExecutorHolder.h"
@@ -47,24 +42,16 @@ namespace facebook::react {
 
 namespace {
 
-class Exception : public jni::JavaClass<Exception> {
- public:
-};
-
 class JInstanceCallback : public InstanceCallback {
  public:
-  explicit JInstanceCallback(
-      alias_ref<ReactCallback::javaobject> jobj,
-      std::shared_ptr<JMessageQueueThread> messageQueueThread)
-      : jobj_(make_global(jobj)),
-        messageQueueThread_(std::move(messageQueueThread)) {}
+  explicit JInstanceCallback(alias_ref<ReactCallback::javaobject> jobj)
+      : jobj_(make_global(jobj)) {}
 
   void onBatchComplete() override {
-    messageQueueThread_->runOnQueue([this] {
-      static auto method = ReactCallback::javaClassStatic()->getMethod<void()>(
-          "onBatchComplete");
-      method(jobj_);
-    });
+    jni::ThreadScope guard;
+    static auto method =
+        ReactCallback::javaClassStatic()->getMethod<void()>("onBatchComplete");
+    method(jobj_);
   }
 
   void incrementPendingJSCalls() override {
@@ -86,7 +73,6 @@ class JInstanceCallback : public InstanceCallback {
 
  private:
   global_ref<ReactCallback::javaobject> jobj_;
-  std::shared_ptr<JMessageQueueThread> messageQueueThread_;
 };
 
 } // namespace
@@ -201,7 +187,7 @@ void CatalystInstanceImpl::initializeBridge(
       moduleMessageQueue_));
 
   instance_->initializeBridge(
-      std::make_unique<JInstanceCallback>(callback, moduleMessageQueue_),
+      std::make_unique<JInstanceCallback>(callback),
       jseh->getExecutorFactory(),
       std::make_unique<JMessageQueueThread>(jsQueue),
       moduleRegistry_);

packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h

@@ -13,7 +13,6 @@
 #include <ReactCommon/RuntimeExecutor.h>
 #include <fbjni/fbjni.h>
 
-#include "CxxModuleWrapper.h"
 #include "JMessageQueueThread.h"
 #include "JRuntimeExecutor.h"
 #include "JRuntimeScheduler.h"