Scope LongLivedObjectCollection per runtime [3/n] (#43410)

Summary: Pull Request resolved: #43410 Changelog: [General] [Breaking] - Make `LongLivedObjectCollection::get` accept a Runtime reference as parameter. # Context Approach 1 as described in [RFC post](https://fb.workplace.com/groups/615693552291894/permalink/1693347124526526/). # This diff * Replace the `LongLivedObjectCollection` singleton with a map from `Runtime -> LongLivedObjectCollection` so that each RN instance has its own collection. * Update MSFT fork accordingly Reviewed By: javache, RSNara Differential Revision: D54649209 fbshipit-source-id: ecd2ab3917843ca82388b7b9cce06c05679f2d60
facebook · Mar 11, 2024 · 86a52cc · 86a52cc
1 parent da21799
commit 86a52cc
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 14 deletions.
diff --git a/packages/react-native/ReactCommon/react/bridging/CallbackWrapper.h b/packages/react-native/ReactCommon/react/bridging/CallbackWrapper.h
@@ -38,7 +38,7 @@ class CallbackWrapper : public LongLivedObject {
       std::shared_ptr<CallInvoker> jsInvoker) {
     auto wrapper = std::shared_ptr<CallbackWrapper>(new CallbackWrapper(
         std::move(callback), runtime, std::move(jsInvoker)));
-    LongLivedObjectCollection::get().add(wrapper);
+    LongLivedObjectCollection::get(runtime).add(wrapper);
     return wrapper;
   }
 

diff --git a/packages/react-native/ReactCommon/react/bridging/LongLivedObject.cpp b/packages/react-native/ReactCommon/react/bridging/LongLivedObject.cpp
@@ -6,13 +6,24 @@
  */
 
 #include "LongLivedObject.h"
+#include <unordered_map>
 
 namespace facebook::react {
 
 // LongLivedObjectCollection
-LongLivedObjectCollection& LongLivedObjectCollection::get() {
-  static LongLivedObjectCollection instance;
-  return instance;
+
+LongLivedObjectCollection& LongLivedObjectCollection::get(
+    jsi::Runtime& runtime) {
+  static std::unordered_map<void*, std::shared_ptr<LongLivedObjectCollection>>
+      instances;
+  void* key = static_cast<void*>(&runtime);
+  auto entry = instances.find(key);
+  if (entry == instances.end()) {
+    entry =
+        instances.emplace(key, std::make_shared<LongLivedObjectCollection>())
+            .first;
+  }
+  return *(entry->second);
 }
 
 void LongLivedObjectCollection::add(std::shared_ptr<LongLivedObject> so) {
@@ -43,7 +54,7 @@ size_t LongLivedObjectCollection::size() const {
 // LongLivedObject
 
 void LongLivedObject::allowRelease() {
-  LongLivedObjectCollection::get().remove(this);
+  LongLivedObjectCollection::get(runtime_).remove(this);
 }
 
 } // namespace facebook::react
diff --git a/packages/react-native/ReactCommon/react/bridging/LongLivedObject.h b/packages/react-native/ReactCommon/react/bridging/LongLivedObject.h
@@ -42,7 +42,7 @@ class LongLivedObject {
  */
 class LongLivedObjectCollection {
  public:
-  static LongLivedObjectCollection& get();
+  static LongLivedObjectCollection& get(jsi::Runtime& runtime);
 
   LongLivedObjectCollection() = default;
   LongLivedObjectCollection(const LongLivedObjectCollection&) = delete;

diff --git a/packages/react-native/ReactCommon/react/bridging/Promise.h b/packages/react-native/ReactCommon/react/bridging/Promise.h
@@ -36,7 +36,7 @@ class AsyncPromise {
 
     auto promiseHolder =
         std::make_shared<PromiseHolder>(rt, promise.asObject(rt));
-    LongLivedObjectCollection::get().add(promiseHolder);
+    LongLivedObjectCollection::get(rt).add(promiseHolder);
 
     // The shared state can retain the promise holder weakly now.
     state_->promiseHolder = promiseHolder;

diff --git a/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp b/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp
@@ -316,7 +316,7 @@ TEST_F(BridgingTest, asyncCallbackInvalidation) {
       [](jsi::Runtime& rt, jsi::Function& f) { f.call(rt, "hello"); });
 
   // LongLivedObjectCollection goes away before callback is executed
-  LongLivedObjectCollection::get().clear();
+  LongLivedObjectCollection::get(rt).clear();
 
   flushQueue();
 

diff --git a/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.h b/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.h
@@ -43,14 +43,14 @@ class BridgingTest : public ::testing::Test {
         rt(*runtime) {}
 
   ~BridgingTest() {
-    LongLivedObjectCollection::get().clear();
+    LongLivedObjectCollection::get(rt).clear();
   }
 
   void TearDown() override {
     flushQueue();
 
     // After flushing the invoker queue, we shouldn't leak memory.
-    EXPECT_EQ(0, LongLivedObjectCollection::get().size());
+    EXPECT_EQ(0, LongLivedObjectCollection::get(rt).size());
   }
 
   jsi::Value eval(const std::string& js) {

diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp
@@ -67,9 +67,11 @@ class BridgelessNativeModuleProxy : public jsi::HostObject {
  */
 
 TurboModuleBinding::TurboModuleBinding(
+    jsi::Runtime& runtime,
     TurboModuleProviderFunctionType&& moduleProvider,
     std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection)
-    : moduleProvider_(std::move(moduleProvider)),
+    : runtime_(runtime),
+      moduleProvider_(std::move(moduleProvider)),
       longLivedObjectCollection_(std::move(longLivedObjectCollection)) {}
 
 void TurboModuleBinding::install(
@@ -85,7 +87,7 @@ void TurboModuleBinding::install(
           jsi::PropNameID::forAscii(runtime, "__turboModuleProxy"),
           1,
           [binding = TurboModuleBinding(
-               std::move(moduleProvider), longLivedObjectCollection)](
+               runtime, std::move(moduleProvider), longLivedObjectCollection)](
               jsi::Runtime& rt,
               const jsi::Value& thisVal,
               const jsi::Value* args,
@@ -102,7 +104,9 @@ void TurboModuleBinding::install(
     bool rnTurboInterop = legacyModuleProvider != nullptr;
     auto turboModuleBinding = legacyModuleProvider
         ? std::make_unique<TurboModuleBinding>(
-              std::move(legacyModuleProvider), longLivedObjectCollection)
+              runtime,
+              std::move(legacyModuleProvider),
+              longLivedObjectCollection)
         : nullptr;
     auto nativeModuleProxy = std::make_shared<BridgelessNativeModuleProxy>(
         std::move(turboModuleBinding));
@@ -119,7 +123,7 @@ TurboModuleBinding::~TurboModuleBinding() {
   if (longLivedObjectCollection_) {
     longLivedObjectCollection_->clear();
   } else {
-    LongLivedObjectCollection::get().clear();
+    LongLivedObjectCollection::get(runtime_).clear();
   }
 }
 

diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.h b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.h
@@ -34,6 +34,7 @@ class TurboModuleBinding {
           nullptr);
 
   TurboModuleBinding(
+      jsi::Runtime& runtime,
       TurboModuleProviderFunctionType&& moduleProvider,
       std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection);
 
@@ -49,6 +50,7 @@ class TurboModuleBinding {
   jsi::Value getModule(jsi::Runtime& runtime, const std::string& moduleName)
       const;
 
+  jsi::Runtime& runtime_;
   TurboModuleProviderFunctionType moduleProvider_;
   std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection_;
 };