Skip to content

Commit

Permalink
Avoid global_ref to javaPart in TurboModuleManager
Browse files Browse the repository at this point in the history 
Summary:
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 fefb6c0 commit 84d0c2e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 27 deletions.
46 changes: 23 additions & 23 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,28 @@ TurboModuleManager::createLegacyModuleProvider() {
}

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

bool isInteropLayerDisabled = !shouldCreateLegacyModules;

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

TurboModuleBinding::install(
runtime,
createTurboModuleProvider(enableSyncVoidMethods),
createLegacyModuleProvider());
createTurboModuleProvider(javaPart, enableSyncVoidMethods),
createLegacyModuleProvider(javaPart));
});
}

Expand Down
9 changes: 5 additions & 4 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 @@ -57,18 +56,20 @@ class TurboModuleManager : public jni::HybridClass<TurboModuleManager> {
std::shared_ptr<ModuleCache> legacyModuleCache_;

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 84d0c2e

Please sign in to comment.