From e861949ccdf3ac6ffdd128ce5b7959a59ff7e0d0 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 28 Mar 2025 12:32:20 -0700 Subject: [PATCH] Make SafeReleaseJniRef more generic (#50376) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/50376 This makes a couple of tweaks to `SafeReleaseJniRef` to let it be used in more places: 1. Add a default ctor for null state 2. Bridge `get()` to return raw JNI ref, for use when calling Java functions directly (compared to current use case of hybrid objects) 3. Keep JNI environment attached to thread longer term, instead of repeated attach/detach, to allow use for higher frequency objects. Changelog: [Internal] Reviewed By: javache Differential Revision: D71933838 fbshipit-source-id: 622cc70d2d7483475406314abcbcf92d3d2ff227 --- .../main/jni/react/jni/SafeReleaseJniRef.cpp | 30 +++++++++++++++++++ .../main/jni/react/jni/SafeReleaseJniRef.h | 13 +++++++- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.cpp diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.cpp new file mode 100644 index 00000000000..7eb5f504d77 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.cpp @@ -0,0 +1,30 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +namespace facebook::react { + +void ensureThreadDurationJNIEnvAttached() { + // Attaching and detaching the thread to the JNI environment may take + // hundreds of microseconds, and we attach to a very limited number of + // threads (in practice, just the Hermes GC), so we only perform + // registration once, detaching before the thread is destroyed. + // + // > In Android 2.0 (Eclair) and higher you can use pthread_key_create() to + // > define a destructor function that will be called before the thread + // > exits, and call DetachCurrentThread() from there. + // https://github.com/facebookincubator/fbjni/blob/7b7efda0d49b956acf1d3307510e3c73fc55b404/cxx/fbjni/detail/Environment.h#L101 + // https://developer.android.com/training/articles/perf-jni#threads + static thread_local std::optional threadScope; + + if (jni::detail::currentOrNull() == nullptr) { + threadScope.emplace(); + } +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.h b/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.h index 65fb1f8502c..af1eb84fa3c 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.h @@ -12,6 +12,12 @@ namespace facebook::react { +/** + * Ensures that the current thread has an attached JNI environment, setting one + * up to be detached at the end of the thread's lifetime if not. + */ +void ensureThreadDurationJNIEnvAttached(); + /** * A wrapper around a JNI reference (e.g. jni::global_ref<>) that makes it safe * to destroy (decrement refcount) from any thread. This wrapping is necessary @@ -29,6 +35,7 @@ class SafeReleaseJniRef { using T = std::remove_reference())>::type; public: + SafeReleaseJniRef() = default; /* explicit */ SafeReleaseJniRef(RefT ref) : ref_(std::move(ref)) {} SafeReleaseJniRef(const SafeReleaseJniRef& other) = default; SafeReleaseJniRef(SafeReleaseJniRef&& other) = default; @@ -37,7 +44,7 @@ class SafeReleaseJniRef { ~SafeReleaseJniRef() { if (ref_) { - jni::ThreadScope ts; + ensureThreadDurationJNIEnvAttached(); ref_.reset(); } } @@ -66,6 +73,10 @@ class SafeReleaseJniRef { return ref_; } + RefT::javaobject get() const { + return ref_.get(); + } + private: RefT ref_; };