Skip to content

Commit

Permalink
Avoid global_ref to javaPart in TurboModuleManager (#43613)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #43613

Storing a global_ref to `jThis` in a JNI hybrid object leads to a reference cycle which cannot be cleaned up by Java GC. The Java object will keep the C++ object alive and vice versa.

We can replace the reference in this class by using the this pointer passed in when `installJSIBindings` is invoked.

Changelog: [Internal]

Differential Revision: D55241235
  • Loading branch information
@javache @facebook-github-bot
javache authored and facebook-github-bot committed Mar 22, 2024
1 parent 0721900 commit 3d4ed66
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 35 deletions.
56 changes: 26 additions & 30 deletions ...act-native/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,28 +93,25 @@ class JMethodDescriptor : public jni::JavaClass<JMethodDescriptor> {
} // namespace

TurboModuleManager::TurboModuleManager(
jni::alias_ref<TurboModuleManager::javaobject> jThis,
RuntimeExecutor runtimeExecutor,
std::shared_ptr<CallInvoker> jsCallInvoker,
std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker,
jni::alias_ref<TurboModuleManagerDelegate::javaobject> delegate)
: javaPart_(jni::make_global(jThis)),
runtimeExecutor_(runtimeExecutor),
jsCallInvoker_(jsCallInvoker),
nativeMethodCallInvoker_(nativeMethodCallInvoker),
: runtimeExecutor_(std::move(runtimeExecutor)),
jsCallInvoker_(std::move(jsCallInvoker)),
nativeMethodCallInvoker_(std::move(nativeMethodCallInvoker)),
delegate_(jni::make_global(delegate)),
turboModuleCache_(std::make_shared<ModuleCache>()),
legacyModuleCache_(std::make_shared<ModuleCache>()) {}

jni::local_ref<TurboModuleManager::jhybriddata> TurboModuleManager::initHybrid(
jni::alias_ref<jhybridobject> jThis,
jni::alias_ref<jhybridobject> /* unused */,
jni::alias_ref<JRuntimeExecutor::javaobject> runtimeExecutor,
jni::alias_ref<CallInvokerHolder::javaobject> jsCallInvokerHolder,
jni::alias_ref<NativeMethodCallInvokerHolder::javaobject>
nativeMethodCallInvokerHolder,
jni::alias_ref<TurboModuleManagerDelegate::javaobject> delegate) {
return makeCxxInstance(
jThis,
runtimeExecutor->cthis()->get(),
jsCallInvokerHolder->cthis()->getCallInvoker(),
nativeMethodCallInvokerHolder->cthis()->getNativeMethodCallInvoker(),
Expand All @@ -130,20 +127,21 @@ void TurboModuleManager::registerNatives() {
}

TurboModuleProviderFunctionType TurboModuleManager::createTurboModuleProvider(
jni::alias_ref<jhybridobject> javaPart,
bool enableSyncVoidMethods) {
return [turboModuleCache_ = std::weak_ptr<ModuleCache>(turboModuleCache_),
jsCallInvoker_ = std::weak_ptr<CallInvoker>(jsCallInvoker_),
nativeMethodCallInvoker_ =
std::weak_ptr<NativeMethodCallInvoker>(nativeMethodCallInvoker_),
delegate_ = jni::make_weak(delegate_),
javaPart_ = jni::make_weak(javaPart_),
weakDelegate = jni::make_weak(delegate_),
weakJavaPart = jni::make_weak(javaPart),
enableSyncVoidMethods](
const std::string& name) -> std::shared_ptr<TurboModule> {
auto turboModuleCache = turboModuleCache_.lock();
auto jsCallInvoker = jsCallInvoker_.lock();
auto nativeMethodCallInvoker = nativeMethodCallInvoker_.lock();
auto delegate = delegate_.lockLocal();
auto javaPart = javaPart_.lockLocal();
auto delegate = weakDelegate.lockLocal();
auto javaPart = weakJavaPart.lockLocal();

if (!turboModuleCache || !jsCallInvoker || !nativeMethodCallInvoker ||
!delegate || !javaPart) {
Expand Down Expand Up @@ -219,20 +217,20 @@ TurboModuleProviderFunctionType TurboModuleManager::createTurboModuleProvider(
};
}

TurboModuleProviderFunctionType
TurboModuleManager::createLegacyModuleProvider() {
TurboModuleProviderFunctionType TurboModuleManager::createLegacyModuleProvider(
jni::alias_ref<jhybridobject> javaPart) {
return [legacyModuleCache_ = std::weak_ptr<ModuleCache>(legacyModuleCache_),
jsCallInvoker_ = std::weak_ptr<CallInvoker>(jsCallInvoker_),
nativeMethodCallInvoker_ =
std::weak_ptr<NativeMethodCallInvoker>(nativeMethodCallInvoker_),
delegate_ = jni::make_weak(delegate_),
javaPart_ = jni::make_weak(javaPart_)](
weakDelegate = jni::make_weak(delegate_),
weakJavaPart = jni::make_weak(javaPart)](
const std::string& name) -> std::shared_ptr<TurboModule> {
auto legacyModuleCache = legacyModuleCache_.lock();
auto jsCallInvoker = jsCallInvoker_.lock();
auto nativeMethodCallInvoker = nativeMethodCallInvoker_.lock();
auto delegate = delegate_.lockLocal();
auto javaPart = javaPart_.lockLocal();
auto delegate = weakDelegate.lockLocal();
auto javaPart = weakJavaPart.lockLocal();

if (!legacyModuleCache || !jsCallInvoker || !nativeMethodCallInvoker ||
!delegate || !javaPart) {
Expand Down Expand Up @@ -313,26 +311,24 @@ TurboModuleManager::createLegacyModuleProvider() {
}

void TurboModuleManager::installJSIBindings(
jni::alias_ref<jhybridobject> javaPart,
bool shouldCreateLegacyModules,
bool enableSyncVoidMethods) {
if (!jsCallInvoker_) {
auto cxxPart = javaPart->cthis();
if (!cxxPart || !cxxPart->jsCallInvoker_) {
return; // Runtime doesn't exist when attached to Chrome debugger.
}

bool isInteropLayerDisabled = !shouldCreateLegacyModules;

runtimeExecutor_([this, isInteropLayerDisabled, enableSyncVoidMethods](
jsi::Runtime& runtime) {
if (isInteropLayerDisabled) {
TurboModuleBinding::install(
runtime, createTurboModuleProvider(enableSyncVoidMethods));
return;
}

cxxPart->runtimeExecutor_([cxxPart,
javaPart = jni::make_global(javaPart),
shouldCreateLegacyModules,
enableSyncVoidMethods](jsi::Runtime& runtime) {
TurboModuleBinding::install(
runtime,
createTurboModuleProvider(enableSyncVoidMethods),
createLegacyModuleProvider());
cxxPart->createTurboModuleProvider(javaPart, enableSyncVoidMethods),
shouldCreateLegacyModules
? cxxPart->createLegacyModuleProvider(javaPart)
: nullptr);
});
}

Expand Down
11 changes: 6 additions & 5 deletions ...react-native/ReactAndroid/src/main/jni/react/turbomodule/ReactCommon/TurboModuleManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class TurboModuleManager : public jni::HybridClass<TurboModuleManager> {
static auto constexpr kJavaDescriptor =
"Lcom/facebook/react/internal/turbomodule/core/TurboModuleManager;";
static jni::local_ref<jhybriddata> initHybrid(
jni::alias_ref<jhybridobject> jThis,
jni::alias_ref<jhybridobject> /* unused */,
jni::alias_ref<JRuntimeExecutor::javaobject> runtimeExecutor,
jni::alias_ref<CallInvokerHolder::javaobject> jsCallInvokerHolder,
jni::alias_ref<NativeMethodCallInvokerHolder::javaobject>
Expand All @@ -38,7 +38,6 @@ class TurboModuleManager : public jni::HybridClass<TurboModuleManager> {

private:
friend HybridBase;
jni::global_ref<TurboModuleManager::javaobject> javaPart_;
RuntimeExecutor runtimeExecutor_;
std::shared_ptr<CallInvoker> jsCallInvoker_;
std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker_;
Expand All @@ -56,19 +55,21 @@ class TurboModuleManager : public jni::HybridClass<TurboModuleManager> {
std::shared_ptr<ModuleCache> turboModuleCache_;
std::shared_ptr<ModuleCache> legacyModuleCache_;

void installJSIBindings(
static void installJSIBindings(
jni::alias_ref<jhybridobject> javaPart,
bool shouldCreateLegacyModules,
bool enableSyncVoidMethods);
explicit TurboModuleManager(
jni::alias_ref<TurboModuleManager::jhybridobject> jThis,
RuntimeExecutor runtimeExecutor,
std::shared_ptr<CallInvoker> jsCallInvoker,
std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker,
jni::alias_ref<TurboModuleManagerDelegate::javaobject> delegate);

TurboModuleProviderFunctionType createTurboModuleProvider(
jni::alias_ref<jhybridobject> javaPart,
bool enableSyncVoidMethods);
TurboModuleProviderFunctionType createLegacyModuleProvider();
TurboModuleProviderFunctionType createLegacyModuleProvider(
jni::alias_ref<jhybridobject> javaPart);
};

} // namespace facebook::react

0 comments on commit 3d4ed66

Please sign in to comment.