From f7f9250f6e90a41e798a8f62738925a2859a751a Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Tue, 9 Jan 2024 12:23:40 -0800 Subject: [PATCH] Allow non-static LongLivedObjectCollections in TurboModuleBinding (#42194) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../react/bridging/LongLivedObject.h | 5 ++--- .../core/ReactCommon/TurboModuleBinding.cpp | 22 +++++++++++++------ .../core/ReactCommon/TurboModuleBinding.h | 11 ++++++++-- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/react-native/ReactCommon/react/bridging/LongLivedObject.h b/packages/react-native/ReactCommon/react/bridging/LongLivedObject.h index 6022529adc3f81..f8ca6671da906e 100644 --- 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> 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 index b03715f560d8c5..e6f22b544b4750 100644 --- 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 #include -#include #include using namespace facebook; @@ -95,13 +94,16 @@ static void defineReadOnlyGlobal( */ TurboModuleBinding::TurboModuleBinding( - TurboModuleProviderFunctionType&& moduleProvider) - : moduleProvider_(std::move(moduleProvider)) {} + TurboModuleProviderFunctionType&& moduleProvider, + std::shared_ptr 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) { runtime.global().setProperty( runtime, "__turboModuleProxy", @@ -109,7 +111,8 @@ void TurboModuleBinding::install( 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( std::make_unique( - 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 index 385ce24d1a2831..0556c697f1e7cd 100644 --- 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 +#include #include #include @@ -28,9 +29,14 @@ class TurboModuleBinding { static void install( jsi::Runtime& runtime, TurboModuleProviderFunctionType&& moduleProvider, - TurboModuleProviderFunctionType&& legacyModuleProvider = nullptr); + TurboModuleProviderFunctionType&& legacyModuleProvider = nullptr, + std::shared_ptr longLivedObjectCollection = + nullptr); + + TurboModuleBinding( + TurboModuleProviderFunctionType&& moduleProvider, + std::shared_ptr longLivedObjectCollection); - TurboModuleBinding(TurboModuleProviderFunctionType&& moduleProvider); virtual ~TurboModuleBinding(); private: @@ -44,6 +50,7 @@ class TurboModuleBinding { const; TurboModuleProviderFunctionType moduleProvider_; + std::shared_ptr longLivedObjectCollection_; }; } // namespace facebook::react