Merge pull request #26328 from ivanarh/master

Android: Fixed problems with memory leaks due to absent ReleaseStringUTFChars calls.

platform/android/dir_access_jandroid.cpp

@@ -31,6 +31,7 @@
 #include "dir_access_jandroid.h"
 #include "core/print_string.h"
 #include "file_access_jandroid.h"
+#include "string_android.h"
 #include "thread_jandroid.h"
 
 jobject DirAccessJAndroid::io = NULL;
@@ -69,7 +70,7 @@ String DirAccessJAndroid::get_next() {
 	if (!str)
 		return "";
 
-	String ret = String::utf8(env->GetStringUTFChars((jstring)str, NULL));
+	String ret = jstring_to_string((jstring)str, env);
 	env->DeleteLocalRef((jobject)str);
 	return ret;
 }

platform/android/java_class_wrapper.cpp

@@ -29,6 +29,7 @@
 /*************************************************************************/
 
 #include "java_class_wrapper.h"
+#include "string_android.h"
 #include "thread_jandroid.h"
 
 bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, const Variant **p_args, int p_argcount, Variant::CallError &r_error, Variant &ret) {
@@ -553,7 +554,7 @@ void JavaClassWrapper::_bind_methods() {
 bool JavaClassWrapper::_get_type_sig(JNIEnv *env, jobject obj, uint32_t &sig, String &strsig) {
 
 	jstring name2 = (jstring)env->CallObjectMethod(obj, Class_getName);
-	String str_type = env->GetStringUTFChars(name2, NULL);
+	String str_type = jstring_to_string(name2, env);
 	env->DeleteLocalRef(name2);
 	uint32_t t = 0;
 
@@ -697,7 +698,7 @@ bool JavaClass::_convert_object_to_variant(JNIEnv *env, jobject obj, Variant &va
 		} break;
 		case ARG_TYPE_STRING: {
 
-			var = String::utf8(env->GetStringUTFChars((jstring)obj, NULL));
+			var = jstring_to_string((jstring)obj, env);
 			return true;
 		} break;
 		case ARG_TYPE_CLASS: {
@@ -1030,7 +1031,7 @@ bool JavaClass::_convert_object_to_variant(JNIEnv *env, jobject obj, Variant &va
 				if (!o)
 					ret.push_back(Variant());
 				else {
-					String val = String::utf8(env->GetStringUTFChars((jstring)o, NULL));
+					String val = jstring_to_string((jstring)o, env);
 					ret.push_back(val);
 				}
 				env->DeleteLocalRef(o);
@@ -1075,7 +1076,7 @@ Ref<JavaClass> JavaClassWrapper::wrap(const String &p_class) {
 		ERR_CONTINUE(!obj);
 
 		jstring name = (jstring)env->CallObjectMethod(obj, getName);
-		String str_method = env->GetStringUTFChars(name, NULL);
+		String str_method = jstring_to_string(name, env);
 		env->DeleteLocalRef(name);
 
 		Vector<String> params;
@@ -1204,7 +1205,7 @@ Ref<JavaClass> JavaClassWrapper::wrap(const String &p_class) {
 		ERR_CONTINUE(!obj);
 
 		jstring name = (jstring)env->CallObjectMethod(obj, Field_getName);
-		String str_field = env->GetStringUTFChars(name, NULL);
+		String str_field = jstring_to_string(name, env);
 		env->DeleteLocalRef(name);
 		int mods = env->CallIntMethod(obj, Field_getModifiers);
 		if ((mods & 0x8) && (mods & 0x10) && (mods & 0x1)) { //static final public!

platform/android/java_glue.cpp

@@ -41,6 +41,7 @@
 #include "main/input_default.h"
 #include "main/main.h"
 #include "os_android.h"
+#include "string_android.h"
 #include "thread_jandroid.h"
 #include <unistd.h>
 
@@ -223,7 +224,7 @@ String _get_class_name(JNIEnv *env, jclass cls, bool *array) {
 		jboolean isarr = env->CallBooleanMethod(cls, isArray);
 		(*array) = isarr ? true : false;
 	}
-	String name = env->GetStringUTFChars(clsName, NULL);
+	String name = jstring_to_string(clsName, env);
 	env->DeleteLocalRef(clsName);
 
 	return name;
@@ -241,7 +242,7 @@ Variant _jobject_to_variant(JNIEnv *env, jobject obj) {
 
 	if (name == "java.lang.String") {
 
-		return String::utf8(env->GetStringUTFChars((jstring)obj, NULL));
+		return jstring_to_string((jstring)obj, env);
 	};
 
 	if (name == "[Ljava.lang.String;") {
@@ -252,7 +253,7 @@ Variant _jobject_to_variant(JNIEnv *env, jobject obj) {
 
 		for (int i = 0; i < stringCount; i++) {
 			jstring string = (jstring)env->GetObjectArrayElement(arr, i);
-			sarr.push_back(String::utf8(env->GetStringUTFChars(string, NULL)));
+			sarr.push_back(jstring_to_string(string, env));
 			env->DeleteLocalRef(string);
 		}
 
@@ -487,7 +488,7 @@ public:
 			case Variant::STRING: {
 
 				jobject o = env->CallObjectMethodA(instance, E->get().method, v);
-				ret = String::utf8(env->GetStringUTFChars((jstring)o, NULL));
+				ret = jstring_to_string((jstring)o, env);
 				env->DeleteLocalRef(o);
 			} break;
 			case Variant::POOL_STRING_ARRAY: {
@@ -634,20 +635,20 @@ static String _get_user_data_dir() {
 
 	JNIEnv *env = ThreadAndroid::get_env();
 	jstring s = (jstring)env->CallObjectMethod(godot_io, _getDataDir);
-	return String(env->GetStringUTFChars(s, NULL));
+	return jstring_to_string(s, env);
 }
 
 static String _get_locale() {
 
 	JNIEnv *env = ThreadAndroid::get_env();
 	jstring s = (jstring)env->CallObjectMethod(godot_io, _getLocale);
-	return String(env->GetStringUTFChars(s, NULL));
+	return jstring_to_string(s, env);
 }
 
 static String _get_clipboard() {
 	JNIEnv *env = ThreadAndroid::get_env();
 	jstring s = (jstring)env->CallObjectMethod(_godot_instance, _getClipboard);
-	return String(env->GetStringUTFChars(s, NULL));
+	return jstring_to_string(s, env);
 }
 
 static void _set_clipboard(const String &p_text) {
@@ -661,7 +662,7 @@ static String _get_model() {
 
 	JNIEnv *env = ThreadAndroid::get_env();
 	jstring s = (jstring)env->CallObjectMethod(godot_io, _getModel);
-	return String(env->GetStringUTFChars(s, NULL));
+	return jstring_to_string(s, env);
 }
 
 static int _get_screen_dpi() {
@@ -674,7 +675,7 @@ static String _get_unique_id() {
 
 	JNIEnv *env = ThreadAndroid::get_env();
 	jstring s = (jstring)env->CallObjectMethod(godot_io, _getUniqueID);
-	return String(env->GetStringUTFChars(s, NULL));
+	return jstring_to_string(s, env);
 }
 
 static void _show_vk(const String &p_existing) {
@@ -694,7 +695,7 @@ static String _get_system_dir(int p_dir) {
 
 	JNIEnv *env = ThreadAndroid::get_env();
 	jstring s = (jstring)env->CallObjectMethod(godot_io, _getSystemDir, p_dir);
-	return String(env->GetStringUTFChars(s, NULL));
+	return jstring_to_string(s, env);
 }
 
 static int _get_gles_version_code() {
@@ -891,12 +892,14 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jo
 	ThreadAndroid::setup_thread();
 
 	const char **cmdline = NULL;
+	jstring *j_cmdline = NULL;
 	int cmdlen = 0;
 	if (p_cmdline) {
 		cmdlen = env->GetArrayLength(p_cmdline);
 		if (cmdlen) {
-			cmdline = (const char **)malloc((env->GetArrayLength(p_cmdline) + 1) * sizeof(const char *));
+			cmdline = (const char **)malloc((cmdlen + 1) * sizeof(const char *));
 			cmdline[cmdlen] = NULL;
+			j_cmdline = (jstring *)malloc(cmdlen * sizeof(jstring));
 
 			for (int i = 0; i < cmdlen; i++) {
 
@@ -904,12 +907,19 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jo
 				const char *rawString = env->GetStringUTFChars(string, 0);
 
 				cmdline[i] = rawString;
+				j_cmdline[i] = string;
 			}
 		}
 	}
 
 	Error err = Main::setup("apk", cmdlen, (char **)cmdline, false);
 	if (cmdline) {
+		if (j_cmdline) {
+			for (int i = 0; i < cmdlen; ++i) {
+				env->ReleaseStringUTFChars(j_cmdline[i], cmdline[i]);
+			}
+			free(j_cmdline);
+		}
 		free(cmdline);
 	}
 
@@ -1313,7 +1323,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_joyhat(JNIEnv *env, j
 
 JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_joyconnectionchanged(JNIEnv *env, jobject obj, jint p_device, jboolean p_connected, jstring p_name) {
 	if (os_android) {
-		String name = env->GetStringUTFChars(p_name, NULL);
+		String name = jstring_to_string(p_name, env);
 		os_android->joy_connection_changed(p_device, p_connected, name);
 	}
 }
@@ -1386,7 +1396,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_audio(JNIEnv *env, jo
 
 JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_singleton(JNIEnv *env, jobject obj, jstring name, jobject p_object) {
 
-	String singname = env->GetStringUTFChars(name, NULL);
+	String singname = jstring_to_string(name, env);
 	JNISingleton *s = memnew(JNISingleton);
 	s->set_instance(env->NewGlobalRef(p_object));
 	jni_singletons[singname] = s;
@@ -1463,21 +1473,21 @@ static const char *get_jni_sig(const String &p_type) {
 
 JNIEXPORT jstring JNICALL Java_org_godotengine_godot_GodotLib_getGlobal(JNIEnv *env, jobject obj, jstring path) {
 
-	String js = env->GetStringUTFChars(path, NULL);
+	String js = jstring_to_string(path, env);
 
 	return env->NewStringUTF(ProjectSettings::get_singleton()->get(js).operator String().utf8().get_data());
 }
 
 JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_method(JNIEnv *env, jobject obj, jstring sname, jstring name, jstring ret, jobjectArray args) {
 
-	String singname = env->GetStringUTFChars(sname, NULL);
+	String singname = jstring_to_string(sname, env);
 
 	ERR_FAIL_COND(!jni_singletons.has(singname));
 
 	JNISingleton *s = jni_singletons.get(singname);
 
-	String mname = env->GetStringUTFChars(name, NULL);
-	String retval = env->GetStringUTFChars(ret, NULL);
+	String mname = jstring_to_string(name, env);
+	String retval = jstring_to_string(ret, env);
 	Vector<Variant::Type> types;
 	String cs = "(";
 
@@ -1486,9 +1496,9 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_method(JNIEnv *env, j
 	for (int i = 0; i < stringCount; i++) {
 
 		jstring string = (jstring)env->GetObjectArrayElement(args, i);
-		const char *rawString = env->GetStringUTFChars(string, 0);
-		types.push_back(get_jni_type(String(rawString)));
-		cs += get_jni_sig(String(rawString));
+		const String rawString = jstring_to_string(string, env);
+		types.push_back(get_jni_type(rawString));
+		cs += get_jni_sig(rawString);
 	}
 
 	cs += ")";
@@ -1511,7 +1521,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_callobject(JNIEnv *en
 	int res = env->PushLocalFrame(16);
 	ERR_FAIL_COND(res != 0);
 
-	String str_method = env->GetStringUTFChars(method, NULL);
+	String str_method = jstring_to_string(method, env);
 
 	int count = env->GetArrayLength(params);
 	Variant *vlist = (Variant *)alloca(sizeof(Variant) * count);
@@ -1543,7 +1553,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_calldeferred(JNIEnv *
 	int res = env->PushLocalFrame(16);
 	ERR_FAIL_COND(res != 0);
 
-	String str_method = env->GetStringUTFChars(method, NULL);
+	String str_method = jstring_to_string(method, env);
 
 	int count = env->GetArrayLength(params);
 	Variant args[VARIANT_ARG_MAX];

platform/android/string_android.h

@@ -0,0 +1,58 @@
+/*************************************************************************/
+/*  string_android.h                                                     */
+/*************************************************************************/
+/*                       This file is part of:                           */
+/*                           GODOT ENGINE                                */
+/*                      https://godotengine.org                          */
+/*************************************************************************/
+/* Copyright (c) 2007-2019 Juan Linietsky, Ariel Manzur.                 */
+/* Copyright (c) 2014-2019 Godot Engine contributors (cf. AUTHORS.md)    */
+/*                                                                       */
+/* Permission is hereby granted, free of charge, to any person obtaining */
+/* a copy of this software and associated documentation files (the       */
+/* "Software"), to deal in the Software without restriction, including   */
+/* without limitation the rights to use, copy, modify, merge, publish,   */
+/* distribute, sublicense, and/or sell copies of the Software, and to    */
+/* permit persons to whom the Software is furnished to do so, subject to */
+/* the following conditions:                                             */
+/*                                                                       */
+/* The above copyright notice and this permission notice shall be        */
+/* included in all copies or substantial portions of the Software.       */
+/*                                                                       */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,       */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF    */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY  */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,  */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE     */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
+/*************************************************************************/
+
+#ifndef STRING_ANDROID_H
+#define STRING_ANDROID_H
+#include "core/ustring.h"
+#include "thread_jandroid.h"
+#include <jni.h>
+
+/**
+ * Converts JNI jstring to Godot String.
+ * @param source Source JNI string. If null an empty string is returned.
+ * @param env JNI environment instance. If null obtained by ThreadAndroid::get_env().
+ * @return Godot string instance.
+ */
+static inline String jstring_to_string(jstring source, JNIEnv *env = NULL) {
+	String result;
+	if (source) {
+		if (!env) {
+			env = ThreadAndroid::get_env();
+		}
+		const char *const source_utf8 = env->GetStringUTFChars(source, NULL);
+		if (source_utf8) {
+			result.parse_utf8(source_utf8);
+			env->ReleaseStringUTFChars(source, source_utf8);
+		}
+	}
+	return result;
+}
+
+#endif // STRING_ANDROID_H