From b800890ec629cc667df31e5b7fbeacb96ea667b0 Mon Sep 17 00:00:00 2001 From: attila Date: Thu, 2 Nov 2023 17:06:40 +0100 Subject: [PATCH] Android: Fix ContentSharer crash on Android 14 --- .../native/juce_JNIHelpers_android.h | 81 ++++++++++++++++--- .../native/juce_ContentSharer_android.cpp | 7 +- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/modules/juce_core/native/juce_JNIHelpers_android.h b/modules/juce_core/native/juce_JNIHelpers_android.h index 54cc785397b5..65cf0c4eb686 100644 --- a/modules/juce_core/native/juce_JNIHelpers_android.h +++ b/modules/juce_core/native/juce_JNIHelpers_android.h @@ -31,8 +31,15 @@ template class LocalRef { public: - LocalRef() noexcept : obj (nullptr) {} - explicit LocalRef (JavaType o) noexcept : obj (o) {} + LocalRef() noexcept = default; + + /* This constructor must not be used to wrap local references that were not created through + JNI, i.e. for native function callback parameters. + */ + explicit LocalRef (JavaType o) noexcept + : LocalRef (o, false) + {} + LocalRef (const LocalRef& other) noexcept : obj (retain (other.obj)) {} LocalRef (LocalRef&& other) noexcept : obj (nullptr) { std::swap (obj, other.obj); } ~LocalRef() { clear(); } @@ -48,31 +55,83 @@ class LocalRef LocalRef& operator= (const LocalRef& other) { - JavaType newObj = retain (other.obj); - clear(); - obj = newObj; + auto tmp = other; + std::swap (tmp.obj, obj); return *this; } - LocalRef& operator= (LocalRef&& other) + LocalRef& operator= (LocalRef&& other) noexcept { - clear(); - std::swap (other.obj, obj); + auto tmp = std::move (other); + std::swap (tmp.obj, obj); return *this; } + bool operator== (std::nullptr_t) const noexcept { return obj == nullptr; } + bool operator!= (std::nullptr_t) const noexcept { return obj != nullptr; } + operator JavaType() const noexcept { return obj; } + JavaType get() const noexcept { return obj; } -private: - JavaType obj; + auto release() + { + return std::exchange (obj, nullptr); + } + + /** Creates a new internal local reference. */ + static auto addOwner (JavaType o) + { + return LocalRef { o, true }; + } + + /** Takes ownership of the passed in local reference, and deletes it when the LocalRef goes out + of scope. + */ + static auto becomeOwner (JavaType o) + { + return LocalRef { o, false }; + } +private: static JavaType retain (JavaType obj) { return obj == nullptr ? nullptr : (JavaType) getEnv()->NewLocalRef (obj); } + + /* We cannot delete local references that were not created by JNI, e.g. references that were + created by the VM and passed into the native function. + + For these references we should use createNewLocalRef = true, which will create a new + local reference that this wrapper is allowed to delete. + + Doing otherwise will result in an "Attempt to remove non-JNI local reference" warning in the + VM, which could even cause crashes in future VM implementations. + */ + LocalRef (JavaType o, bool createNewLocalRef) noexcept + : obj (createNewLocalRef ? retain (o) : o) + {} + + JavaType obj = nullptr; }; +/* Creates a new local reference that shares ownership with the passed in pointer. + + Can be used for wrapping function parameters that were created outside the JNI. +*/ +template +auto addLocalRefOwner (JavaType t) +{ + return LocalRef::addOwner (t); +} + +/* Wraps a local reference and destroys it when it goes out of scope. */ +template +auto becomeLocalRefOwner (JavaType t) +{ + return LocalRef::becomeOwner (t); +} + //============================================================================== template class GlobalRefImpl @@ -846,7 +905,7 @@ namespace javaString ("").get())); for (int i = 0; i < juceArray.size(); ++i) - env->SetObjectArrayElement (result, i, javaString (juceArray [i]).get()); + env->SetObjectArrayElement (result.get(), i, javaString (juceArray [i]).get()); return result; } diff --git a/modules/juce_gui_basics/native/juce_ContentSharer_android.cpp b/modules/juce_gui_basics/native/juce_ContentSharer_android.cpp index 03d4cfd14762..b196b05d128d 100644 --- a/modules/juce_gui_basics/native/juce_ContentSharer_android.cpp +++ b/modules/juce_gui_basics/native/juce_ContentSharer_android.cpp @@ -336,8 +336,8 @@ class ContentSharerGlobalImpl static jobjectArray JNICALL contentSharerGetStreamTypes (JNIEnv*, jobject /*contentProvider*/, jobject uri, jstring mimeTypeFilter) { - return getInstance().getStreamTypes (LocalRef (static_cast (uri)), - LocalRef (static_cast (mimeTypeFilter))); + return getInstance().getStreamTypes (addLocalRefOwner (uri), + addLocalRefOwner (mimeTypeFilter)); } private: @@ -482,7 +482,8 @@ class ContentSharerGlobalImpl if (extension.isEmpty()) return nullptr; - return juceStringArrayToJava (filterMimeTypes (detail::MimeTypeTable::getMimeTypesForFileExtension (extension), juceString (mimeTypeFilter.get()))); + return juceStringArrayToJava (filterMimeTypes (detail::MimeTypeTable::getMimeTypesForFileExtension (extension), + juceString (mimeTypeFilter.get()))).release(); } std::unique_ptr doIntent (const LocalRef& intent,