Fix retain cycle in ComponentNameResolverManager (#43611)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43611

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.

In this class, we didn't seem to need this reference anyway.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D55240490

fbshipit-source-id: 9ba6f5b4b958cb273ad1923332b7b29a75630aea
This commit is contained in:
Pieter De Baets
2024-03-25 08:57:07 -07:00
committed by Facebook GitHub Bot
parent 12d22e5ff1
commit ee3d5f510c
2 changed files with 16 additions and 33 deletions
@@ -5,11 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/
#include <string>
#include <fbjni/fbjni.h>
#include <jsi/jsi.h>
#include "ComponentNameResolverManager.h"
#include <react/renderer/componentregistry/native/NativeComponentRegistryBinding.h>
@@ -18,12 +13,13 @@ namespace facebook::react {
using namespace facebook::jni;
constexpr static auto ComponentNameResolverJavaDescriptor =
"com/facebook/react/uimanager/ComponentNameResolver";
ComponentNameResolverManager::ComponentNameResolverManager(
jni::alias_ref<ComponentNameResolverManager::javaobject> jThis,
RuntimeExecutor runtimeExecutor,
jni::alias_ref<jobject> componentNameResolver)
: javaPart_(jni::make_global(jThis)),
runtimeExecutor_(runtimeExecutor),
: runtimeExecutor_(std::move(runtimeExecutor)),
componentNameResolver_(jni::make_global(componentNameResolver)) {}
jni::local_ref<ComponentNameResolverManager::jhybriddata>
@@ -32,7 +28,7 @@ ComponentNameResolverManager::initHybrid(
jni::alias_ref<JRuntimeExecutor::javaobject> runtimeExecutor,
jni::alias_ref<jobject> componentNameResolver) {
return makeCxxInstance(
jThis, runtimeExecutor->cthis()->get(), componentNameResolver);
runtimeExecutor->cthis()->get(), componentNameResolver);
}
void ComponentNameResolverManager::registerNatives() {
@@ -45,28 +41,23 @@ void ComponentNameResolverManager::registerNatives() {
}
void ComponentNameResolverManager::installJSIBindings() {
runtimeExecutor_([thizz = this](jsi::Runtime& runtime) {
auto viewManagerProvider = [thizz](const std::string& name) -> bool {
if (thizz->componentNames_.size() == 0) {
runtimeExecutor_([this](jsi::Runtime& runtime) {
auto viewManagerProvider = [this](const std::string& name) -> bool {
if (componentNames_.size() == 0) {
static auto getComponentNames =
jni::findClassStatic(ComponentNameResolverManager::
ComponentNameResolverJavaDescriptor)
jni::findClassStatic(ComponentNameResolverJavaDescriptor)
->getMethod<jni::alias_ref<jtypeArray<jstring>>()>(
"getComponentNames");
auto componentNamesJArray =
getComponentNames(thizz->componentNameResolver_.get());
auto componentNamesJArray = getComponentNames(componentNameResolver_);
auto len = componentNamesJArray->size();
for (size_t i = 0; i < len; i++) {
jni::local_ref<jstring> elem = (*componentNamesJArray)[i];
auto componentName = elem->toStdString();
thizz->componentNames_.insert(componentName);
componentNames_.insert(elem->toStdString());
}
}
return thizz->componentNames_.find(name) != thizz->componentNames_.end();
return componentNames_.find(name) != componentNames_.end();
};
bindHasComponentProvider(runtime, std::move(viewManagerProvider));
});
}
@@ -7,11 +7,10 @@
#pragma once
#include <ReactCommon/CallInvokerHolder.h>
#include <ReactCommon/RuntimeExecutor.h>
#include <unordered_set>
#include <fbjni/fbjni.h>
#include <react/jni/JRuntimeExecutor.h>
#include <set>
namespace facebook::react {
@@ -21,9 +20,6 @@ class ComponentNameResolverManager
static auto constexpr kJavaDescriptor =
"Lcom/facebook/react/uimanager/ComponentNameResolverManager;";
constexpr static auto ComponentNameResolverJavaDescriptor =
"com/facebook/react/uimanager/ComponentNameResolver";
static facebook::jni::local_ref<jhybriddata> initHybrid(
facebook::jni::alias_ref<jhybridobject> jThis,
facebook::jni::alias_ref<JRuntimeExecutor::javaobject> runtimeExecutor,
@@ -33,18 +29,14 @@ class ComponentNameResolverManager
private:
friend HybridBase;
facebook::jni::global_ref<ComponentNameResolverManager::javaobject> javaPart_;
RuntimeExecutor runtimeExecutor_;
facebook::jni::global_ref<jobject> componentNameResolver_;
std::set<std::string> componentNames_;
std::unordered_set<std::string> componentNames_;
void installJSIBindings();
explicit ComponentNameResolverManager(
facebook::jni::alias_ref<ComponentNameResolverManager::jhybridobject>
jThis,
RuntimeExecutor runtimeExecutor,
facebook::jni::alias_ref<jobject> componentNameResolver);
};