Pass jsi::Runtime reference into CallInvoker::invoke* callbacks (face…

…book#43375) Summary: ## Changelog: [Internal] - As discussed with the team, it makes more sense to pass the reference to the correct `jsi::Runtime` object as an argument to the ` CallInvoker::invoke*` callbacks, that are provided by the user. There are various use cases when user would like to get a hold of the `jsi::Runtime` in the callback, and it makes sense, since it is guaranteed to run on the JS thread. So far people have been coming up with all kinds of workarounds for that, none of them safe enough. Reviewed By: rubennorte Differential Revision: D54643171
rshest · Mar 8, 2024 · 1c8f223 · 1c8f223
1 parent da91323
commit 1c8f223
Show file tree

Hide file tree

Showing 14 changed files with 77 additions and 63 deletions.
diff --git a/packages/react-native/React/CxxBridge/RCTCxxBridge.mm b/packages/react-native/React/CxxBridge/RCTCxxBridge.mm
@@ -1566,7 +1566,7 @@ - (void *)runtime
   return _reactInstance->getJavaScriptContext();
 }
 
-- (void)invokeAsync:(std::function<void()> &&)func
+- (void)invokeAsync:(CallFunc &&)func
 {
   __block auto retainedFunc = std::move(func);
   __weak __typeof(self) weakSelf = self;

diff --git a/packages/react-native/ReactCommon/callinvoker/ReactCommon/CallInvoker.h b/packages/react-native/ReactCommon/callinvoker/ReactCommon/CallInvoker.h
@@ -12,9 +12,13 @@
 
 #include "SchedulerPriority.h"
 
+namespace facebook::jsi {
+class Runtime;
+}
+
 namespace facebook::react {
 
-using CallFunc = std::function<void()>;
+using CallFunc = std::function<void(jsi::Runtime&)>;
 
 /**
  * An interface for a generic native-to-JS call invoker. See BridgeJSCallInvoker
@@ -31,15 +35,29 @@ class CallInvoker {
     invokeAsync(std::move(func));
   }
   virtual void invokeSync(CallFunc&& func) = 0;
+
+  // Backward compatibility only, prefer the CallFunc methods instead
+  virtual void invokeAsync(std::function<void()>&& func) noexcept {
+    invokeAsync([func](jsi::Runtime&) { func(); });
+  }
+
+  virtual void invokeSync(std::function<void()>&& func) {
+    invokeSync([func](jsi::Runtime&) { func(); });
+  }
+
   virtual ~CallInvoker() {}
 };
 
+using NativeMethodCallFunc = std::function<void()>;
+
 class NativeMethodCallInvoker {
  public:
   virtual void invokeAsync(
       const std::string& methodName,
-      CallFunc&& func) noexcept = 0;
-  virtual void invokeSync(const std::string& methodName, CallFunc&& func) = 0;
+      NativeMethodCallFunc&& func) noexcept = 0;
+  virtual void invokeSync(
+      const std::string& methodName,
+      NativeMethodCallFunc&& func) = 0;
   virtual ~NativeMethodCallInvoker() {}
 };
 

diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.cpp b/packages/react-native/ReactCommon/cxxreact/Instance.cpp
@@ -279,14 +279,13 @@ void Instance::JSCallInvoker::setNativeToJsBridgeAndFlushCalls(
   }
 }
 
-void Instance::JSCallInvoker::invokeSync(std::function<void()>&& work) {
+void Instance::JSCallInvoker::invokeSync(CallFunc&& /*work*/) {
   // TODO: Replace JS Callinvoker with RuntimeExecutor.
   throw std::runtime_error(
       "Synchronous native -> JS calls are currently not supported.");
 }
 
-void Instance::JSCallInvoker::invokeAsync(
-    std::function<void()>&& work) noexcept {
+void Instance::JSCallInvoker::invokeAsync(CallFunc&& work) noexcept {
   std::scoped_lock guard(m_mutex);
 
   /**
@@ -311,12 +310,12 @@ void Instance::JSCallInvoker::invokeAsync(
   scheduleAsync(std::move(work));
 }
 
-void Instance::JSCallInvoker::scheduleAsync(
-    std::function<void()>&& work) noexcept {
+void Instance::JSCallInvoker::scheduleAsync(CallFunc&& work) noexcept {
   if (auto strongNativeToJsBridge = m_nativeToJsBridge.lock()) {
     strongNativeToJsBridge->runOnExecutorQueue(
         [work = std::move(work)](JSExecutor* executor) {
-          work();
+          auto* runtime = (jsi::Runtime*)executor->getJavaScriptContext();
+          work(*runtime);
           executor->flush();
         });
   }

diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.h b/packages/react-native/ReactCommon/cxxreact/Instance.h
@@ -164,15 +164,15 @@ class RN_EXPORT Instance : private jsinspector_modern::InstanceTargetDelegate {
     std::weak_ptr<NativeToJsBridge> m_nativeToJsBridge;
     std::mutex m_mutex;
     bool m_shouldBuffer = true;
-    std::list<std::function<void()>> m_workBuffer;
+    std::list<CallFunc> m_workBuffer;
 
-    void scheduleAsync(std::function<void()>&& work) noexcept;
+    void scheduleAsync(CallFunc&& work) noexcept;
 
    public:
     void setNativeToJsBridgeAndFlushCalls(
         std::weak_ptr<NativeToJsBridge> nativeToJsBridge);
-    void invokeAsync(std::function<void()>&& work) noexcept override;
-    void invokeSync(std::function<void()>&& work) override;
+    void invokeAsync(CallFunc&& work) noexcept override;
+    void invokeSync(CallFunc&& work) override;
   };
 
   std::shared_ptr<JSCallInvoker> jsCallInvoker_ =

diff --git a/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp b/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp
@@ -326,14 +326,14 @@ NativeToJsBridge::getDecoratedNativeMethodCallInvoker(
 
     void invokeAsync(
         const std::string& methodName,
-        std::function<void()>&& func) noexcept override {
+        NativeMethodCallFunc&& func) noexcept override {
       if (auto strongJsToNativeBridge = m_jsToNativeBridge.lock()) {
         strongJsToNativeBridge->recordTurboModuleAsyncMethodCall();
       }
       m_nativeInvoker->invokeAsync(methodName, std::move(func));
     }
 
-    void invokeSync(const std::string& methodName, std::function<void()>&& func)
+    void invokeSync(const std::string& methodName, NativeMethodCallFunc&& func)
         override {
       m_nativeInvoker->invokeSync(methodName, std::move(func));
     }

diff --git a/packages/react-native/ReactCommon/react/bridging/Function.h b/packages/react-native/ReactCommon/react/bridging/Function.h
@@ -64,9 +64,8 @@ class AsyncCallback {
     if (auto wrapper = callback_->wrapper_.lock()) {
       auto fn = [callback = callback_,
                  argsPtr = std::make_shared<std::tuple<Args...>>(
-                     std::make_tuple(std::forward<Args>(args)...))] {
-        callback->apply(std::move(*argsPtr));
-      };
+                     std::make_tuple(std::forward<Args>(args)...))](
+                    jsi::Runtime&) { callback->apply(std::move(*argsPtr)); };
 
       auto& jsInvoker = wrapper->jsInvoker();
       if (priority) {
@@ -85,9 +84,10 @@ class AsyncCallback {
       // Capture callback_ and not wrapper_. If callback_ is deallocated or the
       // JSVM is shutdown before the async task is scheduled, the underlying
       // function will have been deallocated.
-      auto fn = [callback = callback_, callImpl = std::move(callImpl)]() {
+      auto fn = [callback = callback_,
+                 callImpl = std::move(callImpl)](jsi::Runtime& rt) {
         if (auto wrapper2 = callback->wrapper_.lock()) {
-          callImpl(wrapper2->runtime(), wrapper2->callback());
+          callImpl(rt, wrapper2->callback());
         }
       };
 

diff --git a/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.h b/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.h
@@ -17,18 +17,18 @@ namespace facebook::react {
 
 class TestCallInvoker : public CallInvoker {
  public:
-  void invokeAsync(std::function<void()>&& fn) noexcept override {
+  void invokeAsync(CallFunc&& fn) noexcept override {
     queue_.push_back(std::move(fn));
   }
 
-  void invokeSync(std::function<void()>&&) override {
+  void invokeSync(CallFunc&&) override {
     FAIL() << "JSCallInvoker does not support invokeSync()";
   }
 
  private:
   friend class BridgingTest;
 
-  std::list<std::function<void()>> queue_;
+  std::list<CallFunc> queue_;
 };
 
 class BridgingTest : public ::testing::Test {
@@ -63,7 +63,7 @@ class BridgingTest : public ::testing::Test {
 
   void flushQueue() {
     while (!invoker->queue_.empty()) {
-      invoker->queue_.front()();
+      invoker->queue_.front()(*runtime);
       invoker->queue_.pop_front();
       rt.drainMicrotasks(); // Run microtasks every cycle.
     }

diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboCxxModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboCxxModule.cpp
@@ -33,24 +33,23 @@ CxxModule::Callback makeTurboCxxModuleCallback(
       return;
     }
 
-    strongWrapper->jsInvoker().invokeAsync([weakWrapper, args]() {
-      auto strongWrapper2 = weakWrapper.lock();
-      if (!strongWrapper2) {
-        return;
-      }
+    strongWrapper->jsInvoker().invokeAsync(
+        [weakWrapper, args](jsi::Runtime& rt) {
+          auto strongWrapper2 = weakWrapper.lock();
+          if (!strongWrapper2) {
+            return;
+          }
 
-      std::vector<jsi::Value> innerArgs;
-      for (auto& a : args) {
-        innerArgs.push_back(
-            jsi::valueFromDynamic(strongWrapper2->runtime(), a));
-      }
-      strongWrapper2->callback().call(
-          strongWrapper2->runtime(),
-          (const jsi::Value*)innerArgs.data(),
-          innerArgs.size());
+          std::vector<jsi::Value> innerArgs;
+          innerArgs.reserve(args.size());
+          for (auto& a : args) {
+            innerArgs.push_back(jsi::valueFromDynamic(rt, a));
+          }
+          strongWrapper2->callback().call(
+              rt, (const jsi::Value*)innerArgs.data(), innerArgs.size());
 
-      strongWrapper2->destroy();
-    });
+          strongWrapper2->destroy();
+        });
 
     wrapperWasCalled = true;
   };

diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp
@@ -43,22 +43,20 @@ void TurboModule::emitDeviceEvent(
     jsi::Runtime& runtime,
     const std::string& eventName,
     ArgFactory argFactory) {
-  jsInvoker_->invokeAsync([&runtime, eventName, argFactory]() {
-    jsi::Value emitter =
-        runtime.global().getProperty(runtime, "__rctDeviceEventEmitter");
+  jsInvoker_->invokeAsync([eventName, argFactory](jsi::Runtime& rt) {
+    jsi::Value emitter = rt.global().getProperty(rt, "__rctDeviceEventEmitter");
     if (!emitter.isUndefined()) {
-      jsi::Object emitterObject = emitter.asObject(runtime);
+      jsi::Object emitterObject = emitter.asObject(rt);
       // TODO: consider caching these
       jsi::Function emitFunction =
-          emitterObject.getPropertyAsFunction(runtime, "emit");
+          emitterObject.getPropertyAsFunction(rt, "emit");
       std::vector<jsi::Value> args;
-      args.emplace_back(
-          jsi::String::createFromAscii(runtime, eventName.c_str()));
+      args.emplace_back(jsi::String::createFromAscii(rt, eventName.c_str()));
       if (argFactory) {
-        argFactory(runtime, args);
+        argFactory(rt, args);
       }
       emitFunction.callWithThis(
-          runtime, emitterObject, (const jsi::Value*)args.data(), args.size());
+          rt, emitterObject, (const jsi::Value*)args.data(), args.size());
     }
   });
 }

diff --git a/.../react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.cpp b/.../react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerCallInvoker.cpp
@@ -19,14 +19,14 @@ RuntimeSchedulerCallInvoker::RuntimeSchedulerCallInvoker(
 void RuntimeSchedulerCallInvoker::invokeAsync(CallFunc&& func) noexcept {
   if (auto runtimeScheduler = runtimeScheduler_.lock()) {
     runtimeScheduler->scheduleWork(
-        [func = std::move(func)](jsi::Runtime&) { func(); });
+        [func = std::move(func)](jsi::Runtime& rt) { func(rt); });
   }
 }
 
 void RuntimeSchedulerCallInvoker::invokeSync(CallFunc&& func) {
   if (auto runtimeScheduler = runtimeScheduler_.lock()) {
     runtimeScheduler->executeNowOnTheSameThread(
-        [func = std::move(func)](jsi::Runtime&) { func(); });
+        [func = std::move(func)](jsi::Runtime& rt) { func(rt); });
   }
 }
 
@@ -35,7 +35,7 @@ void RuntimeSchedulerCallInvoker::invokeAsync(
     CallFunc&& func) noexcept {
   if (auto runtimeScheduler = runtimeScheduler_.lock()) {
     runtimeScheduler->scheduleTask(
-        priority, [func = std::move(func)](jsi::Runtime&) { func(); });
+        priority, [func = std::move(func)](jsi::Runtime& rt) { func(rt); });
   }
 }
 

diff --git a/packages/react-native/ReactCommon/react/runtime/BridgelessJSCallInvoker.cpp b/packages/react-native/ReactCommon/react/runtime/BridgelessJSCallInvoker.cpp
@@ -15,12 +15,12 @@ BridgelessJSCallInvoker::BridgelessJSCallInvoker(
     RuntimeExecutor runtimeExecutor)
     : runtimeExecutor_(std::move(runtimeExecutor)) {}
 
-void BridgelessJSCallInvoker::invokeAsync(
-    std::function<void()>&& func) noexcept {
-  runtimeExecutor_([func = std::move(func)](jsi::Runtime& runtime) { func(); });
+void BridgelessJSCallInvoker::invokeAsync(CallFunc&& func) noexcept {
+  runtimeExecutor_(
+      [func = std::move(func)](jsi::Runtime& runtime) { func(runtime); });
 }
 
-void BridgelessJSCallInvoker::invokeSync(std::function<void()>&& func) {
+void BridgelessJSCallInvoker::invokeSync(CallFunc&& /*func*/) {
   // TODO: Implement this method. The TurboModule infra doesn't call invokeSync.
   throw std::runtime_error(
       "Synchronous native -> JS calls are currently not supported.");

diff --git a/packages/react-native/ReactCommon/react/runtime/BridgelessJSCallInvoker.h b/packages/react-native/ReactCommon/react/runtime/BridgelessJSCallInvoker.h
@@ -20,8 +20,8 @@ namespace facebook::react {
 class BridgelessJSCallInvoker : public CallInvoker {
  public:
   explicit BridgelessJSCallInvoker(RuntimeExecutor runtimeExecutor);
-  void invokeAsync(std::function<void()>&& func) noexcept override;
-  void invokeSync(std::function<void()>&& func) override;
+  void invokeAsync(CallFunc&& func) noexcept override;
+  void invokeSync(CallFunc&& func) override;
 
  private:
   RuntimeExecutor runtimeExecutor_;

diff --git a/packages/react-native/ReactCommon/react/runtime/BridgelessNativeMethodCallInvoker.cpp b/packages/react-native/ReactCommon/react/runtime/BridgelessNativeMethodCallInvoker.cpp
@@ -15,13 +15,13 @@ BridgelessNativeMethodCallInvoker::BridgelessNativeMethodCallInvoker(
 
 void BridgelessNativeMethodCallInvoker::invokeAsync(
     const std::string& methodName,
-    std::function<void()>&& func) noexcept {
+    NativeMethodCallFunc&& func) noexcept {
   messageQueueThread_->runOnQueue(std::move(func));
 }
 
 void BridgelessNativeMethodCallInvoker::invokeSync(
     const std::string& methodName,
-    std::function<void()>&& func) {
+    NativeMethodCallFunc&& func) {
   messageQueueThread_->runOnQueueSync(std::move(func));
 }
 

diff --git a/packages/react-native/ReactCommon/react/runtime/BridgelessNativeMethodCallInvoker.h b/packages/react-native/ReactCommon/react/runtime/BridgelessNativeMethodCallInvoker.h
@@ -20,8 +20,8 @@ class BridgelessNativeMethodCallInvoker : public NativeMethodCallInvoker {
       std::shared_ptr<MessageQueueThread> messageQueueThread);
   void invokeAsync(
       const std::string& methodName,
-      std::function<void()>&& func) noexcept override;
-  void invokeSync(const std::string& methodName, std::function<void()>&& func)
+      NativeMethodCallFunc&& func) noexcept override;
+  void invokeSync(const std::string& methodName, NativeMethodCallFunc&& func)
       override;
 
  private: