From 0b9351c194c8451576b183971e363ceaf658a448 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 25 Dec 2022 16:30:51 +0100 Subject: [PATCH] Android: Make more meticulous use of DeleteLocalRef If we're in a function that isn't just going to immediately return to Java, leaking local references can lead to problems eventually. --- .../jni/AndroidCommon/AndroidCommon.cpp | 79 +++++++++++++------ Source/Android/jni/GameList/GameFileCache.cpp | 4 +- Source/Android/jni/MainAndroid.cpp | 50 +++++++++--- 3 files changed, 96 insertions(+), 37 deletions(-) diff --git a/Source/Android/jni/AndroidCommon/AndroidCommon.cpp b/Source/Android/jni/AndroidCommon/AndroidCommon.cpp index 06696b7ec6..0a2667b1b6 100644 --- a/Source/Android/jni/AndroidCommon/AndroidCommon.cpp +++ b/Source/Android/jni/AndroidCommon/AndroidCommon.cpp @@ -107,40 +107,64 @@ std::string OpenModeToAndroid(std::ios_base::openmode mode) int OpenAndroidContent(const std::string& uri, const std::string& mode) { JNIEnv* env = IDCache::GetEnvForThread(); - return env->CallStaticIntMethod(IDCache::GetContentHandlerClass(), - IDCache::GetContentHandlerOpenFd(), ToJString(env, uri), - ToJString(env, mode)); + + jstring j_uri = ToJString(env, uri); + jstring j_mode = ToJString(env, mode); + + jint result = env->CallStaticIntMethod(IDCache::GetContentHandlerClass(), + IDCache::GetContentHandlerOpenFd(), j_uri, j_mode); + + env->DeleteLocalRef(j_uri); + env->DeleteLocalRef(j_mode); + + return result; } bool DeleteAndroidContent(const std::string& uri) { JNIEnv* env = IDCache::GetEnvForThread(); - return env->CallStaticBooleanMethod(IDCache::GetContentHandlerClass(), - IDCache::GetContentHandlerDelete(), ToJString(env, uri)); + + jstring j_uri = ToJString(env, uri); + + jboolean result = env->CallStaticBooleanMethod(IDCache::GetContentHandlerClass(), + IDCache::GetContentHandlerDelete(), j_uri); + + env->DeleteLocalRef(j_uri); + + return static_cast(result); } jlong GetAndroidContentSizeAndIsDirectory(const std::string& uri) { JNIEnv* env = IDCache::GetEnvForThread(); - return env->CallStaticLongMethod(IDCache::GetContentHandlerClass(), - IDCache::GetContentHandlerGetSizeAndIsDirectory(), - ToJString(env, uri)); + + jstring j_uri = ToJString(env, uri); + + jlong result = env->CallStaticLongMethod( + IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerGetSizeAndIsDirectory(), j_uri); + + env->DeleteLocalRef(j_uri); + + return result; } std::string GetAndroidContentDisplayName(const std::string& uri) { JNIEnv* env = IDCache::GetEnvForThread(); - jstring jresult = reinterpret_cast( - env->CallStaticObjectMethod(IDCache::GetContentHandlerClass(), - IDCache::GetContentHandlerGetDisplayName(), ToJString(env, uri))); + jstring j_uri = ToJString(env, uri); - if (!jresult) + jstring j_result = reinterpret_cast(env->CallStaticObjectMethod( + IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerGetDisplayName(), j_uri)); + + env->DeleteLocalRef(j_uri); + + if (!j_result) return ""; - std::string result = GetJString(env, jresult); + std::string result = GetJString(env, j_result); - env->DeleteLocalRef(jresult); + env->DeleteLocalRef(j_result); return result; } @@ -149,13 +173,15 @@ std::vector GetAndroidContentChildNames(const std::string& uri) { JNIEnv* env = IDCache::GetEnvForThread(); - jobjectArray jresult = reinterpret_cast(env->CallStaticObjectMethod( - IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerGetChildNames(), - ToJString(env, uri), false)); + jstring j_uri = ToJString(env, uri); - std::vector result = JStringArrayToVector(env, jresult); + jobjectArray j_result = reinterpret_cast(env->CallStaticObjectMethod( + IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerGetChildNames(), j_uri, false)); - env->DeleteLocalRef(jresult); + std::vector result = JStringArrayToVector(env, j_result); + + env->DeleteLocalRef(j_uri); + env->DeleteLocalRef(j_result); return result; } @@ -166,13 +192,18 @@ std::vector DoFileSearchAndroidContent(const std::string& directory { JNIEnv* env = IDCache::GetEnvForThread(); - jobjectArray jresult = reinterpret_cast(env->CallStaticObjectMethod( - IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerDoFileSearch(), - ToJString(env, directory), VectorToJStringArray(env, extensions), recursive)); + jstring j_directory = ToJString(env, directory); + jobjectArray j_extensions = VectorToJStringArray(env, extensions); - std::vector result = JStringArrayToVector(env, jresult); + jobjectArray j_result = reinterpret_cast(env->CallStaticObjectMethod( + IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerDoFileSearch(), j_directory, + j_extensions, recursive)); - env->DeleteLocalRef(jresult); + std::vector result = JStringArrayToVector(env, j_result); + + env->DeleteLocalRef(j_directory); + env->DeleteLocalRef(j_extensions); + env->DeleteLocalRef(j_result); return result; } diff --git a/Source/Android/jni/GameList/GameFileCache.cpp b/Source/Android/jni/GameList/GameFileCache.cpp index 061056cc90..280de59bf9 100644 --- a/Source/Android/jni/GameList/GameFileCache.cpp +++ b/Source/Android/jni/GameList/GameFileCache.cpp @@ -52,7 +52,9 @@ Java_org_dolphinemu_dolphinemu_model_GameFileCache_getAllGames(JNIEnv* env, jobj env->NewObjectArray(static_cast(ptr->GetSize()), IDCache::GetGameFileClass(), nullptr); jsize i = 0; GetPointer(env, obj)->ForEach([env, array, &i](const auto& game_file) { - env->SetObjectArrayElement(array, i++, GameFileToJava(env, game_file)); + jobject j_game_file = GameFileToJava(env, game_file); + env->SetObjectArrayElement(array, i++, j_game_file); + env->DeleteLocalRef(j_game_file); }); return array; } diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index 1e80675d8d..428870d02f 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -204,11 +204,16 @@ static bool MsgAlert(const char* caption, const char* text, bool yes_no, Common: std::thread([&] { JNIEnv* env = IDCache::GetEnvForThread(); + jstring j_caption = ToJString(env, caption); + jstring j_text = ToJString(env, text); + // Execute the Java method. result = env->CallStaticBooleanMethod( - IDCache::GetNativeLibraryClass(), IDCache::GetDisplayAlertMsg(), ToJString(env, caption), - ToJString(env, text), yes_no, style == Common::MsgType::Warning, - s_need_nonblocking_alert_msg); + IDCache::GetNativeLibraryClass(), IDCache::GetDisplayAlertMsg(), j_caption, j_text, yes_no, + style == Common::MsgType::Warning, s_need_nonblocking_alert_msg); + + env->DeleteLocalRef(j_caption); + env->DeleteLocalRef(j_text); }).join(); return result != JNI_FALSE; @@ -222,20 +227,29 @@ static void ReportSend(const std::string& endpoint, const std::string& report) jbyte* output = env->GetByteArrayElements(output_array, nullptr); memcpy(output, report.data(), report.size()); env->ReleaseByteArrayElements(output_array, output, 0); + + jstring j_endpoint = ToJString(env, endpoint); + env->CallStaticVoidMethod(IDCache::GetAnalyticsClass(), IDCache::GetSendAnalyticsReport(), - ToJString(env, endpoint), output_array); + j_endpoint, output_array); + + env->DeleteLocalRef(output_array); + env->DeleteLocalRef(j_endpoint); } static std::string GetAnalyticValue(const std::string& key) { JNIEnv* env = IDCache::GetEnvForThread(); - auto value = reinterpret_cast(env->CallStaticObjectMethod( - IDCache::GetAnalyticsClass(), IDCache::GetAnalyticsValue(), ToJString(env, key))); + jstring j_key = ToJString(env, key); + auto j_value = reinterpret_cast(env->CallStaticObjectMethod( + IDCache::GetAnalyticsClass(), IDCache::GetAnalyticsValue(), j_key)); + env->DeleteLocalRef(j_key); - std::string stdvalue = GetJString(env, value); + std::string value = GetJString(env, j_value); + env->DeleteLocalRef(j_value); - return stdvalue; + return value; } extern "C" { @@ -655,8 +669,15 @@ JNIEXPORT jobject JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetLogTyp env->NewObject(IDCache::GetLinkedHashMapClass(), IDCache::GetLinkedHashMapInit(), map_size); for (const auto& entry : map) { - env->CallObjectMethod(linked_hash_map, IDCache::GetLinkedHashMapPut(), - ToJString(env, entry.first), ToJString(env, entry.second)); + jstring key = ToJString(env, entry.first); + jstring value = ToJString(env, entry.second); + + jobject result = + env->CallObjectMethod(linked_hash_map, IDCache::GetLinkedHashMapPut(), key, value); + + env->DeleteLocalRef(key); + env->DeleteLocalRef(value); + env->DeleteLocalRef(result); } return linked_hash_map; } @@ -693,8 +714,13 @@ JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ConvertD const auto callback = [&jCallbackGlobal](const std::string& text, float completion) { JNIEnv* env = IDCache::GetEnvForThread(); - return static_cast(env->CallBooleanMethod( - jCallbackGlobal, IDCache::GetCompressCallbackRun(), ToJString(env, text), completion)); + + jstring j_text = ToJString(env, text); + jboolean result = env->CallBooleanMethod(jCallbackGlobal, IDCache::GetCompressCallbackRun(), + j_text, completion); + env->DeleteLocalRef(j_text); + + return static_cast(result); }; bool success = false;