From e38036e670d234b36053e03be92dd8f27acb16c0 Mon Sep 17 00:00:00 2001 From: Joel Klinghed Date: Thu, 22 Aug 2024 22:30:25 +0200 Subject: samba: Cleanup jni layer Remember that jni::GlobalRef can be used on multiple threads so we can't store the env, need to call AttachCurrentThread. Fix AttachCurrentThread (g_vm was forgotten). More consistent move and copy constructors in refs. Removed env() method as it would only be valid for Param and Local anyway. --- libs/samba/src/main/cpp/jni.cpp | 17 ++++++++ libs/samba/src/main/cpp/jni.hpp | 85 +++++++++++++++++++++++++++++---------- libs/samba/src/main/cpp/samba.cpp | 11 +++++ 3 files changed, 91 insertions(+), 22 deletions(-) (limited to 'libs') diff --git a/libs/samba/src/main/cpp/jni.cpp b/libs/samba/src/main/cpp/jni.cpp index aac1d28..5a69dc5 100644 --- a/libs/samba/src/main/cpp/jni.cpp +++ b/libs/samba/src/main/cpp/jni.cpp @@ -79,6 +79,22 @@ void _abort_with_exception(const char* file, int line, JNIEnv* env) { } } +JNIEnv* NonFatalAttachCurrentThread() { +#ifdef ANDROID + JNIEnv* env; + auto ret = g_vm->AttachCurrentThread(&env, nullptr); + if (ret == JNI_OK) + return env; + return nullptr; +#else + void* v_env; + auto ret = g_vm->AttachCurrentThread(&v_env, nullptr); + if (ret == JNI_OK) + return reinterpret_cast(v_env); + return nullptr; +#endif +} + } // namespace internal JNIEnv* AttachCurrentThread() { @@ -99,6 +115,7 @@ JNIEnv* OnLoad(JavaVM* vm) { void* v_env; auto ret = vm->GetEnv(&v_env, JNI_VERSION); ABORT_IF_NOT_OK(ret); + g_vm = vm; return reinterpret_cast(v_env); } diff --git a/libs/samba/src/main/cpp/jni.hpp b/libs/samba/src/main/cpp/jni.hpp index 450fa71..3b5078a 100644 --- a/libs/samba/src/main/cpp/jni.hpp +++ b/libs/samba/src/main/cpp/jni.hpp @@ -15,28 +15,30 @@ namespace internal { void _abort_if_not_ok(const char *file, int line, jint ret); void _abort_with_exception(const char* file, int line, JNIEnv* env); +JNIEnv* NonFatalAttachCurrentThread(); + } // namespace internal template class Ref { public: - constexpr Ref() : env_(nullptr), ptr_(0) {} + constexpr Ref() : ptr_(nullptr) {} + constexpr Ref(std::nullptr_t) : ptr_(nullptr) {} Ref(const Ref&) = delete; - [[nodiscard]] JNIEnv* env() const { return env_; } [[nodiscard]] T get() const { return ptr_; } [[nodiscard]] T release() { auto ret = release_to_local(); - ptr_ = 0; + ptr_ = nullptr; return ret; } void reset() { del(); - ptr_ = 0; + ptr_ = nullptr; } - explicit operator bool() const { return ptr_ != 0; } + explicit operator bool() const { return ptr_ != nullptr; } protected: Ref(JNIEnv* env, T ptr): env_(env), ptr_(ptr) {} @@ -49,9 +51,17 @@ class Ref { T ptr_; }; +constexpr jint JNI_VERSION = JNI_VERSION_1_2; + +JNIEnv* AttachCurrentThread(); + +JNIEnv* OnLoad(JavaVM* vm); + template class LocalRef : public Ref { public: + constexpr LocalRef(): Ref() {} + constexpr LocalRef(std::nullptr_t): Ref() {} LocalRef(JNIEnv* env, T ptr): Ref(env, ptr) {} ~LocalRef() override { free(); } @@ -59,6 +69,13 @@ class LocalRef : public Ref { LocalRef& operator=(const Ref& other) = delete; + LocalRef& operator=(LocalRef &&ref) noexcept { + free(); + this->env_ = ref.env_; + this->ptr_ = ref.release(); + return *this; + } + protected: T release_to_local() override { return this->ptr_; } void del() override { free(); } @@ -82,7 +99,7 @@ class ParamRef : public Ref { T release_to_local() override { if (this->ptr_) return static_cast(this->env_->NewLocalRef(static_cast(this->ptr_))); - return 0; + return nullptr; } void del() override {} }; @@ -90,43 +107,67 @@ class ParamRef : public Ref { template class GlobalRef : public Ref { public: - GlobalRef(JNIEnv* env, T ptr) : Ref(env, ptr ? static_cast(env->NewGlobalRef(static_cast(ptr))) : 0) {} - explicit GlobalRef(const Ref& other) : Ref(other.env(), other ? static_cast(other.env()->NewGlobalRef(static_cast(other.get()))) : 0) {} + constexpr GlobalRef() : Ref() {} + constexpr GlobalRef(std::nullptr_t) : Ref() {} + GlobalRef(JNIEnv* env, T ptr) : Ref() { + if (ptr) { + if (!env) + env = AttachCurrentThread(); + this->ptr_ = static_cast(env->NewGlobalRef(static_cast(ptr))); + } + } + GlobalRef(const Ref& other) : Ref() { + if (other) { + auto* env = AttachCurrentThread(); + this->ptr_ = static_cast(env->NewGlobalRef(static_cast(other.get()))); + } + } + GlobalRef(GlobalRef&& other) noexcept : Ref(nullptr, other.ptr_) { + other.ptr_ = nullptr; + return *this; + } ~GlobalRef() override { free(); } GlobalRef& operator=(const Ref& other) { free(); - this->env_ = other.env(); - this->ptr_ = other ? static_cast(this->env_->NewGlobalRef(static_cast(other.get()))) : 0; + if (other) { + auto* env = AttachCurrentThread(); + this->ptr_ = other ? static_cast(env->NewGlobalRef(static_cast(other.get()))) : nullptr; + } else { + this->ptr_ = nullptr; + } return *this; } + GlobalRef& operator=(GlobalRef&& other) { + free(); + this->ptr_ = other.ptr_; + other.ptr_ = nullptr; + } + protected: T release_to_local() override { if (this->ptr_) { - auto ret = static_cast(this->env_->NewLocalRef( - static_cast(this->ptr_))); - free(); + auto* env = AttachCurrentThread(); + auto ret = static_cast(env->NewLocalRef(static_cast(this->ptr_))); + env->DeleteGlobalRef(static_cast(this->ptr_)); return ret; } - return 0; + return nullptr; } void del() override { free(); } private: void free() { - if (this->ptr_) - this->env_->DeleteGlobalRef(this->ptr_); + if (this->ptr_) { + auto* env = internal::NonFatalAttachCurrentThread(); + if (env) + env->DeleteGlobalRef(static_cast(this->ptr_)); + } } }; -constexpr jint JNI_VERSION = JNI_VERSION_1_2; - -JNIEnv* AttachCurrentThread(); - -JNIEnv* OnLoad(JavaVM* vm); - LocalRef FindClass(JNIEnv *env, const char *name); LocalRef ExceptionOccurred(JNIEnv* env); diff --git a/libs/samba/src/main/cpp/samba.cpp b/libs/samba/src/main/cpp/samba.cpp index 1d1d9d2..1e6b425 100644 --- a/libs/samba/src/main/cpp/samba.cpp +++ b/libs/samba/src/main/cpp/samba.cpp @@ -260,6 +260,12 @@ void RegisterSamba(JNIEnv* env) { g_DirEntryClass = dir_entry_clazz; } +void UnregisterSamba() { + g_CreateDirEntry = 0; + g_NativeSambaClass.reset(); + g_DirEntryClass.reset(); +} + jni::LocalRef CreateDirEntry(JNIEnv* env, const std::string& name, const smb2_stat_64& stat) { auto j_name = jni::UTF8ToString(env, name); // Kotlin size casts Long to ULong @@ -292,3 +298,8 @@ jint JNI_OnLoad(JavaVM *vm, void *reserved) { return jni::JNI_VERSION; } + +void JNI_OnUnload(JavaVM *vm, void *reserved) { + // Not called on Android (or in general), but if it where, this would be the place to unregister. + UnregisterSamba(); +} -- cgit v1.2.3-70-g09d2