Allow non-static LongLivedObjectCollections in TurboModuleBinding (#4…

…2194) Summary: Pull Request resolved: #42194 Apps that have multiple concurrently running React instances may suffer from issues where tearing down one instance affects the bindings / LongLivedObjectCollection instance of another due to the use of static getter for LongLivedObjectCollection. This should allow host platforms, e.g., react-native-windows (which still forks the TurboModuleBinding C++ files [here](https://github.com/microsoft/react-native-windows/tree/main/vnext/ReactCommon/TEMP_UntilReactCommonUpdate/react/nativemodule/core/ReactCommon) for the reasons already mentioned) to manage per instance LongLivedObjectCollections. ## Changelog [Internal] Reviewed By: christophpurrer Differential Revision: D52581170 fbshipit-source-id: 791e3baeefaf23f544eeddd5a216735535523a9d
facebook · Jan 9, 2024 · f7f9250 · f7f9250
1 parent e4f878d
commit f7f9250
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 12 deletions.
diff --git a/packages/react-native/ReactCommon/react/bridging/LongLivedObject.h b/packages/react-native/ReactCommon/react/bridging/LongLivedObject.h
@@ -28,7 +28,7 @@ namespace facebook::react {
  */
 class LongLivedObject {
  public:
-  void allowRelease();
+  virtual void allowRelease();
 
  protected:
   LongLivedObject() = default;
@@ -42,6 +42,7 @@ class LongLivedObjectCollection {
  public:
   static LongLivedObjectCollection& get();
 
+  LongLivedObjectCollection() = default;
   LongLivedObjectCollection(const LongLivedObjectCollection&) = delete;
   void operator=(const LongLivedObjectCollection&) = delete;
 
@@ -51,8 +52,6 @@ class LongLivedObjectCollection {
   size_t size() const;
 
  private:
-  LongLivedObjectCollection() = default;
-
   std::unordered_set<std::shared_ptr<LongLivedObject>> collection_;
   mutable std::mutex collectionMutex_;
 };

diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp
@@ -10,7 +10,6 @@
 #include <stdexcept>
 #include <string>
 
-#include <ReactCommon/LongLivedObject.h>
 #include <cxxreact/SystraceSection.h>
 
 using namespace facebook;
@@ -95,21 +94,25 @@ static void defineReadOnlyGlobal(
  */
 
 TurboModuleBinding::TurboModuleBinding(
-    TurboModuleProviderFunctionType&& moduleProvider)
-    : moduleProvider_(std::move(moduleProvider)) {}
+    TurboModuleProviderFunctionType&& moduleProvider,
+    std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection)
+    : moduleProvider_(std::move(moduleProvider)),
+      longLivedObjectCollection_(std::move(longLivedObjectCollection)) {}
 
 void TurboModuleBinding::install(
     jsi::Runtime& runtime,
     TurboModuleProviderFunctionType&& moduleProvider,
-    TurboModuleProviderFunctionType&& legacyModuleProvider) {
+    TurboModuleProviderFunctionType&& legacyModuleProvider,
+    std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection) {
   runtime.global().setProperty(
       runtime,
       "__turboModuleProxy",
       jsi::Function::createFromHostFunction(
           runtime,
           jsi::PropNameID::forAscii(runtime, "__turboModuleProxy"),
           1,
-          [binding = TurboModuleBinding(std::move(moduleProvider))](
+          [binding = TurboModuleBinding(
+               std::move(moduleProvider), longLivedObjectCollection)](
               jsi::Runtime& rt,
               const jsi::Value& thisVal,
               const jsi::Value* args,
@@ -132,7 +135,8 @@ void TurboModuleBinding::install(
               runtime,
               std::make_shared<BridgelessNativeModuleProxy>(
                   std::make_unique<TurboModuleBinding>(
-                      std::move(legacyModuleProvider)))));
+                      std::move(legacyModuleProvider),
+                      longLivedObjectCollection))));
     } else {
       defineReadOnlyGlobal(
           runtime,
@@ -144,7 +148,11 @@ void TurboModuleBinding::install(
 }
 
 TurboModuleBinding::~TurboModuleBinding() {
-  LongLivedObjectCollection::get().clear();
+  if (longLivedObjectCollection_) {
+    longLivedObjectCollection_->clear();
+  } else {
+    LongLivedObjectCollection::get().clear();
+  }
 }
 
 jsi::Value TurboModuleBinding::getModule(

diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.h b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.h
@@ -9,6 +9,7 @@
 
 #include <string>
 
+#include <ReactCommon/LongLivedObject.h>
 #include <ReactCommon/TurboModule.h>
 #include <jsi/jsi.h>
 
@@ -28,9 +29,14 @@ class TurboModuleBinding {
   static void install(
       jsi::Runtime& runtime,
       TurboModuleProviderFunctionType&& moduleProvider,
-      TurboModuleProviderFunctionType&& legacyModuleProvider = nullptr);
+      TurboModuleProviderFunctionType&& legacyModuleProvider = nullptr,
+      std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection =
+          nullptr);
+
+  TurboModuleBinding(
+      TurboModuleProviderFunctionType&& moduleProvider,
+      std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection);
 
-  TurboModuleBinding(TurboModuleProviderFunctionType&& moduleProvider);
   virtual ~TurboModuleBinding();
 
  private:
@@ -44,6 +50,7 @@ class TurboModuleBinding {
       const;
 
   TurboModuleProviderFunctionType moduleProvider_;
+  std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection_;
 };
 
 } // namespace facebook::react