Skip to content

Commit

Permalink
Allow non-static LongLivedObjectCollections in TurboModuleBinding (#4…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
rozele authored and facebook-github-bot committed Jan 9, 2024
1 parent e4f878d commit f7f9250
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace facebook::react {
*/
class LongLivedObject {
public:
void allowRelease();
virtual void allowRelease();

protected:
LongLivedObject() = default;
Expand All @@ -42,6 +42,7 @@ class LongLivedObjectCollection {
public:
static LongLivedObjectCollection& get();

LongLivedObjectCollection() = default;
LongLivedObjectCollection(const LongLivedObjectCollection&) = delete;
void operator=(const LongLivedObjectCollection&) = delete;

Expand All @@ -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_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <stdexcept>
#include <string>

#include <ReactCommon/LongLivedObject.h>
#include <cxxreact/SystraceSection.h>

using namespace facebook;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -144,7 +148,11 @@ void TurboModuleBinding::install(
}

TurboModuleBinding::~TurboModuleBinding() {
LongLivedObjectCollection::get().clear();
if (longLivedObjectCollection_) {
longLivedObjectCollection_->clear();
} else {
LongLivedObjectCollection::get().clear();
}
}

jsi::Value TurboModuleBinding::getModule(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <string>

#include <ReactCommon/LongLivedObject.h>
#include <ReactCommon/TurboModule.h>
#include <jsi/jsi.h>

Expand All @@ -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:
Expand All @@ -44,6 +50,7 @@ class TurboModuleBinding {
const;

TurboModuleProviderFunctionType moduleProvider_;
std::shared_ptr<LongLivedObjectCollection> longLivedObjectCollection_;
};

} // namespace facebook::react

0 comments on commit f7f9250

Please sign in to comment.