Merge pull request #11376 from JosJuice/android-delete-local-ref

Android: Make more meticulous use of DeleteLocalRef

Source/Android/jni/AndroidCommon/AndroidCommon.cpp

@@ -20,7 +20,7 @@ std::string GetJString(JNIEnv* env, jstring jstr)
   const jchar* jchars = env->GetStringChars(jstr, nullptr);
   const jsize length = env->GetStringLength(jstr);
   const std::u16string_view string_view(reinterpret_cast<const char16_t*>(jchars), length);
-  const std::string converted_string = UTF16ToUTF8(string_view);
+  std::string converted_string = UTF16ToUTF8(string_view);
   env->ReleaseStringChars(jstr, jchars);
   return converted_string;
 }
@@ -48,16 +48,9 @@ std::vector<std::string> JStringArrayToVector(JNIEnv* env, jobjectArray array)
   return result;
 }
 
-jobjectArray VectorToJStringArray(JNIEnv* env, std::vector<std::string> vector)
+jobjectArray VectorToJStringArray(JNIEnv* env, const std::vector<std::string>& vector)
 {
-  jobjectArray result = env->NewObjectArray(vector.size(), IDCache::GetStringClass(), nullptr);
-  for (jsize i = 0; i < vector.size(); ++i)
-  {
-    jstring str = ToJString(env, vector[i]);
-    env->SetObjectArrayElement(result, i, str);
-    env->DeleteLocalRef(str);
-  }
-  return result;
+  return VectorToJObjectArray(env, vector, IDCache::GetStringClass(), ToJString);
 }
 
 bool IsPathAndroidContent(const std::string& uri)
@@ -107,40 +100,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<bool>(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<jstring>(
-      env->CallStaticObjectMethod(IDCache::GetContentHandlerClass(),
-                                  IDCache::GetContentHandlerGetDisplayName(), ToJString(env, uri)));
+  jstring j_uri = ToJString(env, uri);
+
+  jstring j_result = reinterpret_cast<jstring>(env->CallStaticObjectMethod(
+      IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerGetDisplayName(), j_uri));
+
+  env->DeleteLocalRef(j_uri);
 
-  if (!jresult)
+  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 +166,15 @@ std::vector<std::string> GetAndroidContentChildNames(const std::string& uri)
 {
   JNIEnv* env = IDCache::GetEnvForThread();
 
-  jobjectArray jresult = reinterpret_cast<jobjectArray>(env->CallStaticObjectMethod(
-      IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerGetChildNames(),
-      ToJString(env, uri), false));
+  jstring j_uri = ToJString(env, uri);
 
-  std::vector<std::string> result = JStringArrayToVector(env, jresult);
+  jobjectArray j_result = reinterpret_cast<jobjectArray>(env->CallStaticObjectMethod(
+      IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerGetChildNames(), j_uri, false));
 
-  env->DeleteLocalRef(jresult);
+  std::vector<std::string> result = JStringArrayToVector(env, j_result);
+
+  env->DeleteLocalRef(j_uri);
+  env->DeleteLocalRef(j_result);
 
   return result;
 }
@@ -166,13 +185,18 @@ std::vector<std::string> DoFileSearchAndroidContent(const std::string& directory
 {
   JNIEnv* env = IDCache::GetEnvForThread();
 
-  jobjectArray jresult = reinterpret_cast<jobjectArray>(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);
+
+  jobjectArray j_result = reinterpret_cast<jobjectArray>(env->CallStaticObjectMethod(
+      IDCache::GetContentHandlerClass(), IDCache::GetContentHandlerDoFileSearch(), j_directory,
+      j_extensions, recursive));
 
-  std::vector<std::string> result = JStringArrayToVector(env, jresult);
+  std::vector<std::string> result = JStringArrayToVector(env, j_result);
 
-  env->DeleteLocalRef(jresult);
+  env->DeleteLocalRef(j_directory);
+  env->DeleteLocalRef(j_extensions);
+  env->DeleteLocalRef(j_result);
 
   return result;
 }

Source/Android/jni/AndroidCommon/AndroidCommon.h

@@ -13,7 +13,20 @@ std::string GetJString(JNIEnv* env, jstring jstr);
 jstring ToJString(JNIEnv* env, const std::string& str);
 
 std::vector<std::string> JStringArrayToVector(JNIEnv* env, jobjectArray array);
-jobjectArray VectorToJStringArray(JNIEnv* env, std::vector<std::string> vector);
+jobjectArray VectorToJStringArray(JNIEnv* env, const std::vector<std::string>& vector);
+
+template <typename T, typename F>
+jobjectArray VectorToJObjectArray(JNIEnv* env, const std::vector<T>& vector, jclass clazz, F f)
+{
+  jobjectArray result = env->NewObjectArray(vector.size(), clazz, nullptr);
+  for (jsize i = 0; i < vector.size(); ++i)
+  {
+    jobject obj = f(env, vector[i]);
+    env->SetObjectArrayElement(result, i, obj);
+    env->DeleteLocalRef(obj);
+  }
+  return result;
+}
 
 // Returns true if the given path should be opened as Android content instead of a normal file.
 bool IsPathAndroidContent(const std::string& uri);

Source/Android/jni/Cheats/ARCheat.cpp

@@ -147,14 +147,7 @@ Java_org_dolphinemu_dolphinemu_features_cheats_model_ARCheat_loadCodes(JNIEnv* e
   const std::vector<ActionReplay::ARCode> codes =
       ActionReplay::LoadCodes(game_ini_default, game_ini_local);
 
-  const jobjectArray array =
-      env->NewObjectArray(static_cast<jsize>(codes.size()), IDCache::GetARCheatClass(), nullptr);
-
-  jsize i = 0;
-  for (const ActionReplay::ARCode& code : codes)
-    env->SetObjectArrayElement(array, i++, ARCheatToJava(env, code));
-
-  return array;
+  return VectorToJObjectArray(env, codes, IDCache::GetARCheatClass(), ARCheatToJava);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_features_cheats_model_ARCheat_saveCodes(

Source/Android/jni/Cheats/GeckoCheat.cpp

@@ -155,14 +155,7 @@ Java_org_dolphinemu_dolphinemu_features_cheats_model_GeckoCheat_loadCodes(JNIEnv
 
   const std::vector<Gecko::GeckoCode> codes = Gecko::LoadCodes(game_ini_default, game_ini_local);
 
-  const jobjectArray array =
-      env->NewObjectArray(static_cast<jsize>(codes.size()), IDCache::GetGeckoCheatClass(), nullptr);
-
-  jsize i = 0;
-  for (const Gecko::GeckoCode& code : codes)
-    env->SetObjectArrayElement(array, i++, GeckoCheatToJava(env, code));
-
-  return array;
+  return VectorToJObjectArray(env, codes, IDCache::GetGeckoCheatClass(), GeckoCheatToJava);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_features_cheats_model_GeckoCheat_saveCodes(

Source/Android/jni/Cheats/GraphicsModGroup.cpp

@@ -62,14 +62,9 @@ Java_org_dolphinemu_dolphinemu_features_cheats_model_GraphicsModGroup_getMods(JN
     mods.push_back(&mod);
   }
 
-  const jobjectArray array =
-      env->NewObjectArray(static_cast<jsize>(mods.size()), IDCache::GetGraphicsModClass(), nullptr);
-
-  jsize i = 0;
-  for (GraphicsModConfig* mod : mods)
-    env->SetObjectArrayElement(array, i++, GraphicsModToJava(env, mod, obj));
-
-  return array;
+  return VectorToJObjectArray(
+      env, mods, IDCache::GetGraphicsModClass(),
+      [obj](JNIEnv* env, GraphicsModConfig* mod) { return GraphicsModToJava(env, mod, obj); });
 }
 
 JNIEXPORT void JNICALL

Source/Android/jni/Cheats/PatchCheat.cpp

@@ -134,14 +134,7 @@ Java_org_dolphinemu_dolphinemu_features_cheats_model_PatchCheat_loadCodes(JNIEnv
   std::vector<PatchEngine::Patch> patches;
   PatchEngine::LoadPatchSection("OnFrame", &patches, game_ini_default, game_ini_local);
 
-  const jobjectArray array = env->NewObjectArray(static_cast<jsize>(patches.size()),
-                                                 IDCache::GetPatchCheatClass(), nullptr);
-
-  jsize i = 0;
-  for (const PatchEngine::Patch& patch : patches)
-    env->SetObjectArrayElement(array, i++, PatchCheatToJava(env, patch));
-
-  return array;
+  return VectorToJObjectArray(env, patches, IDCache::GetPatchCheatClass(), PatchCheatToJava);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_features_cheats_model_PatchCheat_saveCodes(

Source/Android/jni/GameList/GameFileCache.cpp

@@ -52,7 +52,9 @@ Java_org_dolphinemu_dolphinemu_model_GameFileCache_getAllGames(JNIEnv* env, jobj
       env->NewObjectArray(static_cast<jsize>(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;
 }

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<jstring>(env->CallStaticObjectMethod(
-      IDCache::GetAnalyticsClass(), IDCache::GetAnalyticsValue(), ToJString(env, key)));
+  jstring j_key = ToJString(env, key);
+  auto j_value = reinterpret_cast<jstring>(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<bool>(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<bool>(result);
   };
 
   bool success = false;