From 0622cee1896b2a939464ba98330589b4e36053b0 Mon Sep 17 00:00:00 2001 From: Fredia Huya-Kouadio Date: Thu, 29 May 2025 12:40:18 -0700 Subject: [PATCH 1/2] Implement `has_java_method(...)` for `JavaClassWrapper` and `JNISingleton` --- doc/classes/JNISingleton.xml | 9 +++ doc/classes/JavaClass.xml | 7 +++ doc/classes/JavaObject.xml | 7 +++ platform/android/api/api.cpp | 10 +++ platform/android/api/java_class_wrapper.h | 6 +- platform/android/api/jni_singleton.h | 63 +++++++++++++------ .../godot/plugin/AndroidRuntimePlugin.kt | 2 +- platform/android/java_class_wrapper.cpp | 60 +++++++++++++++--- 8 files changed, 134 insertions(+), 30 deletions(-) diff --git a/doc/classes/JNISingleton.xml b/doc/classes/JNISingleton.xml index 041b8600b0e..50271ac1fc9 100644 --- a/doc/classes/JNISingleton.xml +++ b/doc/classes/JNISingleton.xml @@ -9,4 +9,13 @@ $DOCS_URL/tutorials/platform/android/android_plugin.html#doc-android-plugin + + + + + + Returns [code]true[/code] if the given [param method] name exists in the JNISingleton's Java methods. + + + diff --git a/doc/classes/JavaClass.xml b/doc/classes/JavaClass.xml index 9a6c30df10e..0fea241a0e8 100644 --- a/doc/classes/JavaClass.xml +++ b/doc/classes/JavaClass.xml @@ -29,5 +29,12 @@ Returns a [JavaClass] representing the Java parent class of this class. + + + + + Returns [code]true[/code] if the given [param method] name exists in the object's Java methods. + + diff --git a/doc/classes/JavaObject.xml b/doc/classes/JavaObject.xml index f38070e7d9d..aa090a809b9 100644 --- a/doc/classes/JavaObject.xml +++ b/doc/classes/JavaObject.xml @@ -17,5 +17,12 @@ Returns the [JavaClass] that this object is an instance of. + + + + + Returns [code]true[/code] if the given [param method] name exists in the object's Java methods. + + diff --git a/platform/android/api/api.cpp b/platform/android/api/api.cpp index 86051a87f3b..8c4a0779953 100644 --- a/platform/android/api/api.cpp +++ b/platform/android/api/api.cpp @@ -62,10 +62,12 @@ void JavaClass::_bind_methods() { ClassDB::bind_method(D_METHOD("get_java_class_name"), &JavaClass::get_java_class_name); ClassDB::bind_method(D_METHOD("get_java_method_list"), &JavaClass::get_java_method_list); ClassDB::bind_method(D_METHOD("get_java_parent_class"), &JavaClass::get_java_parent_class); + ClassDB::bind_method(D_METHOD("has_java_method", "method"), &JavaClass::has_java_method); } void JavaObject::_bind_methods() { ClassDB::bind_method(D_METHOD("get_java_class"), &JavaObject::get_java_class); + ClassDB::bind_method(D_METHOD("has_java_method", "method"), &JavaObject::has_java_method); } void JavaClassWrapper::_bind_methods() { @@ -94,6 +96,10 @@ Ref JavaClass::get_java_parent_class() const { return Ref(); } +bool JavaClass::has_java_method(const StringName &) const { + return false; +} + JavaClass::JavaClass() { } @@ -108,6 +114,10 @@ Ref JavaObject::get_java_class() const { return Ref(); } +bool JavaObject::has_java_method(const StringName &) const { + return false; +} + JavaClassWrapper *JavaClassWrapper::singleton = nullptr; Ref JavaClassWrapper::_wrap(const String &, bool) { diff --git a/platform/android/api/java_class_wrapper.h b/platform/android/api/java_class_wrapper.h index de04c80723b..e0f59df2248 100644 --- a/platform/android/api/java_class_wrapper.h +++ b/platform/android/api/java_class_wrapper.h @@ -200,6 +200,7 @@ public: String get_java_class_name() const; TypedArray get_java_method_list() const; Ref get_java_parent_class() const; + bool has_java_method(const StringName &p_method) const; #ifdef ANDROID_ENABLED virtual String to_string() override; @@ -226,6 +227,7 @@ public: virtual Variant callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) override; Ref get_java_class() const; + bool has_java_method(const StringName &p_method) const; #ifdef ANDROID_ENABLED virtual String to_string() override; @@ -244,7 +246,7 @@ class JavaClassWrapper : public Object { #ifdef ANDROID_ENABLED RBMap> class_cache; friend class JavaClass; - jmethodID Class_getDeclaredConstructors; + jmethodID Class_getConstructors; jmethodID Class_getDeclaredMethods; jmethodID Class_getFields; jmethodID Class_getName; @@ -272,7 +274,7 @@ class JavaClassWrapper : public Object { Ref exception; - Ref _wrap(const String &p_class, bool p_allow_private_methods_access); + Ref _wrap(const String &p_class, bool p_allow_non_public_methods_access); static JavaClassWrapper *singleton; diff --git a/platform/android/api/jni_singleton.h b/platform/android/api/jni_singleton.h index 63df40b1114..64c1e8bf6ad 100644 --- a/platform/android/api/jni_singleton.h +++ b/platform/android/api/jni_singleton.h @@ -46,35 +46,60 @@ class JNISingleton : public Object { RBMap method_map; Ref wrapped_object; +protected: + static void _bind_methods() { + ClassDB::bind_method(D_METHOD("has_java_method", "method"), &JNISingleton::has_java_method); + } + public: virtual Variant callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) override { - if (wrapped_object.is_valid()) { - RBMap::Element *E = method_map.find(p_method); - - // Check the method we're looking for is in the JNISingleton map and that - // the arguments match. - bool call_error = !E || E->get().argtypes.size() != p_argcount; - if (!call_error) { - for (int i = 0; i < p_argcount; i++) { - if (!Variant::can_convert(p_args[i]->get_type(), E->get().argtypes[i])) { - call_error = true; - break; - } - } - } - - if (!call_error) { - return wrapped_object->callp(p_method, p_args, p_argcount, r_error); - } + // Godot methods take precedence. + Variant ret = Object::callp(p_method, p_args, p_argcount, r_error); + if (r_error.error == Callable::CallError::CALL_OK) { + return ret; } - return Object::callp(p_method, p_args, p_argcount, r_error); + // Check the method we're looking for is in the JNISingleton map. + RBMap::Element *E = method_map.find(p_method); + if (E) { + if (wrapped_object.is_valid()) { + // Check that the arguments match. + int method_arg_count = E->get().argtypes.size(); + if (p_argcount < method_arg_count) { + r_error.error = Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS; + } else if (p_argcount > method_arg_count) { + r_error.error = Callable::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS; + } else { + // Check the arguments are valid. + bool arguments_valid = true; + for (int i = 0; i < p_argcount; i++) { + if (!Variant::can_convert(p_args[i]->get_type(), E->get().argtypes[i])) { + arguments_valid = false; + break; + } + } + + if (arguments_valid) { + return wrapped_object->callp(p_method, p_args, p_argcount, r_error); + } else { + r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT; + } + } + } else { + r_error.error = Callable::CallError::CALL_ERROR_INSTANCE_IS_NULL; + } + } + return Variant(); } Ref get_wrapped_object() const { return wrapped_object; } + bool has_java_method(const StringName &p_method) const { + return method_map.has(p_method); + } + void add_method(const StringName &p_name, const Vector &p_args, Variant::Type p_ret_type) { MethodData md; md.argtypes = p_args; diff --git a/platform/android/java/lib/src/org/godotengine/godot/plugin/AndroidRuntimePlugin.kt b/platform/android/java/lib/src/org/godotengine/godot/plugin/AndroidRuntimePlugin.kt index 35ff7d6fa14..ca1c32cce9e 100644 --- a/platform/android/java/lib/src/org/godotengine/godot/plugin/AndroidRuntimePlugin.kt +++ b/platform/android/java/lib/src/org/godotengine/godot/plugin/AndroidRuntimePlugin.kt @@ -51,7 +51,7 @@ class AndroidRuntimePlugin(godot: Godot) : GodotPlugin(godot) { * Provides access to the host [android.app.Activity] to GDScript */ @UsedByGodot - override fun getActivity() = super.getActivity() + public override fun getActivity() = super.getActivity() /** * Utility method used to create [Runnable] from Godot [Callable]. diff --git a/platform/android/java_class_wrapper.cpp b/platform/android/java_class_wrapper.cpp index 8339ff6ae3a..78607c5565c 100644 --- a/platform/android/java_class_wrapper.cpp +++ b/platform/android/java_class_wrapper.cpp @@ -36,6 +36,7 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error, Variant &ret) { HashMap>::Iterator M = methods.find(p_method); if (!M) { + r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD; return false; } @@ -756,7 +757,11 @@ bool JavaClass::_get(const StringName &p_name, Variant &r_ret) const { } Variant JavaClass::callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) { - Variant ret; + // Godot methods take precedence. + Variant ret = RefCounted::callp(p_method, p_args, p_argcount, r_error); + if (r_error.error == Callable::CallError::CALL_OK) { + return ret; + } String method = (p_method == java_constructor_name) ? "" : p_method; bool found = _call_method(nullptr, method, p_args, p_argcount, r_error, ret); @@ -764,7 +769,7 @@ Variant JavaClass::callp(const StringName &p_method, const Variant **p_args, int return ret; } - return RefCounted::callp(p_method, p_args, p_argcount, r_error); + return Variant(); } String JavaClass::get_java_class_name() const { @@ -858,6 +863,11 @@ String JavaClass::to_string() { return ""; } +bool JavaClass::has_java_method(const StringName &p_method) const { + String method = (p_method == java_constructor_name) ? "" : p_method; + return methods.has(method); +} + JavaClass::JavaClass() { } @@ -873,10 +883,15 @@ JavaClass::~JavaClass() { ///////////////////// Variant JavaObject::callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) { + // Godot methods take precedence. + Variant ret = RefCounted::callp(p_method, p_args, p_argcount, r_error); + if (r_error.error == Callable::CallError::CALL_OK) { + return ret; + } + if (instance) { Ref c = base_class; while (c.is_valid()) { - Variant ret; bool found = c->_call_method(this, p_method, p_args, p_argcount, r_error, ret); if (found) { return ret; @@ -885,7 +900,7 @@ Variant JavaObject::callp(const StringName &p_method, const Variant **p_args, in } } - return RefCounted::callp(p_method, p_args, p_argcount, r_error); + return Variant(); } Ref JavaObject::get_java_class() const { @@ -899,6 +914,19 @@ String JavaObject::to_string() { return RefCounted::to_string(); } +bool JavaObject::has_java_method(const StringName &p_method) const { + if (instance) { + Ref c = base_class; + while (c.is_valid()) { + if (c->has_java_method(p_method)) { + return true; + } + c = c->get_java_parent_class(); + } + } + return false; +} + JavaObject::JavaObject() { } @@ -1461,7 +1489,7 @@ bool JavaClass::_convert_object_to_variant(JNIEnv *env, jobject obj, Variant &va return false; } -Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_private_methods_access) { +Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_non_public_methods_access) { String class_name_dots = p_class.replace_char('/', '.'); if (class_cache.has(class_name_dots)) { return class_cache[class_name_dots]; @@ -1473,10 +1501,18 @@ Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_priva jclass bclass = jni_find_class(env, class_name_dots.replace_char('.', '/').utf8().get_data()); ERR_FAIL_NULL_V_MSG(bclass, Ref(), vformat("Java class '%s' not found.", p_class)); - jobjectArray constructors = (jobjectArray)env->CallObjectMethod(bclass, Class_getDeclaredConstructors); + jobjectArray constructors = (jobjectArray)env->CallObjectMethod(bclass, Class_getConstructors); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + env->ExceptionClear(); + } ERR_FAIL_NULL_V(constructors, Ref()); jobjectArray methods = (jobjectArray)env->CallObjectMethod(bclass, Class_getDeclaredMethods); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + env->ExceptionClear(); + } ERR_FAIL_NULL_V(methods, Ref()); Ref java_class = memnew(JavaClass); @@ -1515,8 +1551,9 @@ Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_priva Vector params; jint mods = env->CallIntMethod(obj, is_constructor ? Constructor_getModifiers : Method_getModifiers); + bool is_public = (mods & 0x0001) != 0; // java.lang.reflect.Modifier.PUBLIC - if (!(mods & 0x0001) && (is_constructor || !p_allow_private_methods_access)) { + if (!is_public && (is_constructor || !p_allow_non_public_methods_access)) { env->DeleteLocalRef(obj); continue; //not public bye } @@ -1627,6 +1664,13 @@ Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_priva mi.method = env->GetMethodID(bclass, str_method.utf8().get_data(), signature.utf8().get_data()); } + if (env->ExceptionCheck()) { + // Exceptions may be thrown when trying to access hidden methods; write the exception to the logs and continue. + env->ExceptionDescribe(); + env->ExceptionClear(); + continue; + } + ERR_CONTINUE(!mi.method); java_class->methods[str_method].push_back(mi); @@ -1700,7 +1744,7 @@ JavaClassWrapper::JavaClassWrapper() { ERR_FAIL_NULL(env); jclass bclass = jni_find_class(env, "java/lang/Class"); - Class_getDeclaredConstructors = env->GetMethodID(bclass, "getDeclaredConstructors", "()[Ljava/lang/reflect/Constructor;"); + Class_getConstructors = env->GetMethodID(bclass, "getConstructors", "()[Ljava/lang/reflect/Constructor;"); Class_getDeclaredMethods = env->GetMethodID(bclass, "getDeclaredMethods", "()[Ljava/lang/reflect/Method;"); Class_getFields = env->GetMethodID(bclass, "getFields", "()[Ljava/lang/reflect/Field;"); Class_getName = env->GetMethodID(bclass, "getName", "()Ljava/lang/String;"); From 35fda7f857a8e55cc65959a310a4327bacf5f63a Mon Sep 17 00:00:00 2001 From: Fredia Huya-Kouadio Date: Sat, 16 Aug 2025 22:09:20 -0700 Subject: [PATCH 2/2] Fix JNI local reference table overflow when wrapping Java class with large method counts --- platform/android/java_class_wrapper.cpp | 68 +++----------- platform/android/java_godot_lib_jni.cpp | 4 +- platform/android/jni_utils.cpp | 109 +++++++++++----------- platform/android/jni_utils.h | 8 +- platform/android/variant/callable_jni.cpp | 8 +- 5 files changed, 77 insertions(+), 120 deletions(-) diff --git a/platform/android/java_class_wrapper.cpp b/platform/android/java_class_wrapper.cpp index 78607c5565c..e2d223a6b18 100644 --- a/platform/android/java_class_wrapper.cpp +++ b/platform/android/java_class_wrapper.cpp @@ -43,6 +43,8 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, JNIEnv *env = get_jni_env(); ERR_FAIL_NULL_V(env, false); + env->PushLocalFrame(p_argcount); + MethodInfo *method = nullptr; for (MethodInfo &E : M->value) { if (!p_instance && !E._static && !E._constructor) { @@ -284,7 +286,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, argv = (jvalue *)alloca(sizeof(jvalue) * method->param_types.size()); } - List to_free; for (int i = 0; i < method->param_types.size(); i++) { switch (method->param_types[i]) { case ARG_TYPE_VOID: { @@ -323,7 +324,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.z = (bool)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_NUMBER_CLASS_BIT | ARG_TYPE_BYTE: { jclass bclass = jni_find_class(env, "java/lang/Byte"); @@ -332,7 +332,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.b = (int)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_NUMBER_CLASS_BIT | ARG_TYPE_CHAR: { jclass bclass = jni_find_class(env, "java/lang/Character"); @@ -341,7 +340,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.c = (int)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_NUMBER_CLASS_BIT | ARG_TYPE_SHORT: { jclass bclass = jni_find_class(env, "java/lang/Short"); @@ -350,7 +348,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.s = (int)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_NUMBER_CLASS_BIT | ARG_TYPE_INT: { jclass bclass = jni_find_class(env, "java/lang/Integer"); @@ -359,7 +356,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.i = (int)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_NUMBER_CLASS_BIT | ARG_TYPE_LONG: { jclass bclass = jni_find_class(env, "java/lang/Long"); @@ -368,7 +364,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.j = (int64_t)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_NUMBER_CLASS_BIT | ARG_TYPE_FLOAT: { jclass bclass = jni_find_class(env, "java/lang/Float"); @@ -377,7 +372,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.f = (float)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_NUMBER_CLASS_BIT | ARG_TYPE_DOUBLE: { jclass bclass = jni_find_class(env, "java/lang/Double"); @@ -386,23 +380,20 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, val.d = (double)(*p_args[i]); jobject obj = env->NewObjectA(bclass, ctor, &val); argv[i].l = obj; - to_free.push_back(obj); } break; case ARG_TYPE_STRING: case ARG_TYPE_CHARSEQUENCE: { String s = *p_args[i]; jstring jStr = env->NewStringUTF(s.utf8().get_data()); argv[i].l = jStr; - to_free.push_back(jStr); } break; case ARG_TYPE_CALLABLE: { jobject jcallable = callable_to_jcallable(env, *p_args[i]); argv[i].l = jcallable; - to_free.push_back(jcallable); } break; case ARG_TYPE_CLASS: { if (p_args[i]->get_type() == Variant::DICTIONARY) { - argv[i].l = _variant_to_jvalue(env, Variant::DICTIONARY, p_args[i]).obj; + argv[i].l = _variant_to_jvalue(env, Variant::DICTIONARY, p_args[i]).l; } else { Ref jo = *p_args[i]; if (jo.is_valid()) { @@ -420,8 +411,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, env->SetBooleanArrayRegion(a, j, 1, &val); } argv[i].l = a; - to_free.push_back(a); - } break; case ARG_ARRAY_BIT | ARG_TYPE_BYTE: { jbyteArray a = nullptr; @@ -440,8 +429,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = a; - to_free.push_back(a); - } break; case ARG_ARRAY_BIT | ARG_TYPE_CHAR: { jcharArray a = nullptr; @@ -462,7 +449,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = a; - to_free.push_back(a); } break; case ARG_ARRAY_BIT | ARG_TYPE_SHORT: { @@ -485,7 +471,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = a; - to_free.push_back(a); } break; case ARG_ARRAY_BIT | ARG_TYPE_INT: { @@ -505,7 +490,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = a; - to_free.push_back(a); } break; case ARG_ARRAY_BIT | ARG_TYPE_LONG: { jlongArray a = nullptr; @@ -524,8 +508,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = a; - to_free.push_back(a); - } break; case ARG_ARRAY_BIT | ARG_TYPE_FLOAT: { jfloatArray a = nullptr; @@ -544,8 +526,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = a; - to_free.push_back(a); - } break; case ARG_ARRAY_BIT | ARG_TYPE_DOUBLE: { jdoubleArray a = nullptr; @@ -564,8 +544,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = a; - to_free.push_back(a); - } break; case ARG_ARRAY_BIT | ARG_TYPE_STRING: case ARG_ARRAY_BIT | ARG_TYPE_CHARSEQUENCE: { @@ -578,7 +556,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, String s = arr[j]; jstring jStr = env->NewStringUTF(s.utf8().get_data()); env->SetObjectArrayElement(a, j, jStr); - to_free.push_back(jStr); } } else if (p_args[i]->get_type() == Variant::PACKED_STRING_ARRAY) { PackedStringArray arr = *p_args[i]; @@ -587,12 +564,10 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, String s = arr[j]; jstring jStr = env->NewStringUTF(s.utf8().get_data()); env->SetObjectArrayElement(a, j, jStr); - to_free.push_back(jStr); } } argv[i].l = a; - to_free.push_back(a); } break; case ARG_ARRAY_BIT | ARG_TYPE_CALLABLE: { Array arr = *p_args[i]; @@ -601,11 +576,9 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, Variant callable = arr[j]; jobject jcallable = callable_to_jcallable(env, callable); env->SetObjectArrayElement(jarr, j, jcallable); - to_free.push_back(jcallable); } argv[i].l = jarr; - to_free.push_back(jarr); } break; case ARG_ARRAY_BIT | ARG_TYPE_CLASS: { String cn = method->param_sigs[i].operator String(); @@ -622,7 +595,6 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, } argv[i].l = jarr; - to_free.push_back(jarr); } } break; } @@ -720,30 +692,25 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method, r_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD; success = false; } - env->DeleteLocalRef(obj); } } break; } - for (jobject &E : to_free) { - env->DeleteLocalRef(E); - } - jobject exception = env->ExceptionOccurred(); if (exception) { env->ExceptionClear(); jclass java_class = env->GetObjectClass(exception); Ref java_class_wrapped = JavaClassWrapper::singleton->wrap_jclass(java_class); - env->DeleteLocalRef(java_class); JavaClassWrapper::singleton->exception.instantiate(java_class_wrapped, exception); - env->DeleteLocalRef(exception); } else { JavaClassWrapper::singleton->exception.unref(); } + env->PopLocalFrame(nullptr); + return success; } @@ -1522,22 +1489,16 @@ Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_non_p java_class->_class = (jclass)env->NewGlobalRef(bclass); class_cache[class_name_dots] = java_class; - LocalVector methods_and_constructors; int constructor_count = env->GetArrayLength(constructors); int method_count = env->GetArrayLength(methods); - methods_and_constructors.resize(method_count + constructor_count); - for (int i = 0; i < constructor_count; i++) { - methods_and_constructors[i] = env->GetObjectArrayElement(constructors, i); - } - for (int i = 0; i < method_count; i++) { - methods_and_constructors[constructor_count + i] = env->GetObjectArrayElement(methods, i); - } - - for (int i = 0; i < (int)methods_and_constructors.size(); i++) { - jobject obj = methods_and_constructors[i]; - ERR_CONTINUE(!obj); + int methods_and_constructors_count = method_count + constructor_count; + for (int i = 0; i < methods_and_constructors_count; i++) { bool is_constructor = i < constructor_count; + jobject obj = is_constructor + ? env->GetObjectArrayElement(constructors, i) + : env->GetObjectArrayElement(methods, i - constructor_count); + ERR_CONTINUE(!obj); String str_method; if (is_constructor) { @@ -1671,9 +1632,9 @@ Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_non_p continue; } - ERR_CONTINUE(!mi.method); - - java_class->methods[str_method].push_back(mi); + if (mi.method) { + java_class->methods[str_method].push_back(mi); + } } env->DeleteLocalRef(obj); @@ -1720,6 +1681,7 @@ Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_non_p } env->DeleteLocalRef(fields); + env->DeleteLocalRef(bclass); return java_class; } diff --git a/platform/android/java_godot_lib_jni.cpp b/platform/android/java_godot_lib_jni.cpp index c7a549f1a85..1643c9b329d 100644 --- a/platform/android/java_godot_lib_jni.cpp +++ b/platform/android/java_godot_lib_jni.cpp @@ -514,7 +514,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setEditorSetting(JNIE } JNIEXPORT jobject JNICALL Java_org_godotengine_godot_GodotLib_getEditorProjectMetadata(JNIEnv *env, jclass clazz, jstring p_section, jstring p_key, jobject p_default_value) { - jvalret result; + jvalue result; #ifdef TOOLS_ENABLED if (EditorSettings::get_singleton() != nullptr) { @@ -528,7 +528,7 @@ JNIEXPORT jobject JNICALL Java_org_godotengine_godot_GodotLib_getEditorProjectMe WARN_PRINT("Access to the Editor Settings Project Metadata is only available on Editor builds"); #endif - return result.obj; + return result.l; } JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setEditorProjectMetadata(JNIEnv *env, jclass clazz, jstring p_section, jstring p_key, jobject p_data) { diff --git a/platform/android/jni_utils.cpp b/platform/android/jni_utils.cpp index 2bf1add4860..1753c7388a7 100644 --- a/platform/android/jni_utils.cpp +++ b/platform/android/jni_utils.cpp @@ -86,15 +86,16 @@ String charsequence_to_string(JNIEnv *p_env, jobject p_charsequence) { return result; } -jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_arg, bool force_jobject, int p_depth) { - jvalret v; +jvalue _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_arg, bool force_jobject, int p_depth) { + jvalue value; if (p_depth > Variant::MAX_RECURSION_DEPTH) { ERR_PRINT("Variant is too deep! Bailing."); - v.val.i = 0; - return v; + value.i = 0; + return value; } + env->PushLocalFrame(2); switch (p_type) { case Variant::BOOL: { if (force_jobject) { @@ -103,11 +104,10 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jvalue val; val.z = (bool)(*p_arg); jobject obj = env->NewObjectA(bclass, ctor, &val); - v.val.l = obj; - v.obj = obj; + value.l = obj; env->DeleteLocalRef(bclass); } else { - v.val.z = *p_arg; + value.z = *p_arg; } } break; case Variant::INT: { @@ -117,12 +117,11 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jvalue val; val.i = (int)(*p_arg); jobject obj = env->NewObjectA(bclass, ctor, &val); - v.val.l = obj; - v.obj = obj; + value.l = obj; env->DeleteLocalRef(bclass); } else { - v.val.i = *p_arg; + value.i = *p_arg; } } break; case Variant::FLOAT: { @@ -132,19 +131,17 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jvalue val; val.d = (double)(*p_arg); jobject obj = env->NewObjectA(bclass, ctor, &val); - v.val.l = obj; - v.obj = obj; + value.l = obj; env->DeleteLocalRef(bclass); } else { - v.val.f = *p_arg; + value.f = *p_arg; } } break; case Variant::STRING: { String s = *p_arg; jstring jStr = env->NewStringUTF(s.utf8().get_data()); - v.val.l = jStr; - v.obj = jStr; + value.l = jStr; } break; case Variant::PACKED_STRING_ARRAY: { Vector sarray = *p_arg; @@ -155,15 +152,13 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a env->SetObjectArrayElement(arr, j, str); env->DeleteLocalRef(str); } - v.val.l = arr; - v.obj = arr; + value.l = arr; } break; case Variant::CALLABLE: { jobject jcallable = callable_to_jcallable(env, *p_arg); - v.val.l = jcallable; - v.obj = jcallable; + value.l = jcallable; } break; case Variant::DICTIONARY: { @@ -191,10 +186,10 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a for (int j = 0; j < keys.size(); j++) { Variant var = dict[keys[j]]; - jvalret valret = _variant_to_jvalue(env, var.get_type(), &var, true, p_depth + 1); - env->SetObjectArrayElement(jvalues, j, valret.val.l); - if (valret.obj) { - env->DeleteLocalRef(valret.obj); + jvalue valret = _variant_to_jvalue(env, var.get_type(), &var, true, p_depth + 1); + env->SetObjectArrayElement(jvalues, j, valret.l); + if (valret.l) { + env->DeleteLocalRef(valret.l); } } @@ -204,8 +199,7 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a env->DeleteLocalRef(jvalues); env->DeleteLocalRef(dclass); - v.val.l = jdict; - v.obj = jdict; + value.l = jdict; } break; case Variant::ARRAY: { @@ -214,14 +208,13 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a for (int j = 0; j < array.size(); j++) { Variant var = array[j]; - jvalret valret = _variant_to_jvalue(env, var.get_type(), &var, true, p_depth + 1); - env->SetObjectArrayElement(arr, j, valret.val.l); - if (valret.obj) { - env->DeleteLocalRef(valret.obj); + jvalue valret = _variant_to_jvalue(env, var.get_type(), &var, true, p_depth + 1); + env->SetObjectArrayElement(arr, j, valret.l); + if (valret.l) { + env->DeleteLocalRef(valret.l); } } - v.val.l = arr; - v.obj = arr; + value.l = arr; } break; case Variant::PACKED_INT32_ARRAY: { @@ -229,8 +222,7 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jintArray arr = env->NewIntArray(array.size()); const int *r = array.ptr(); env->SetIntArrayRegion(arr, 0, array.size(), r); - v.val.l = arr; - v.obj = arr; + value.l = arr; } break; case Variant::PACKED_INT64_ARRAY: { @@ -238,8 +230,7 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jlongArray arr = env->NewLongArray(array.size()); const int64_t *r = array.ptr(); env->SetLongArrayRegion(arr, 0, array.size(), r); - v.val.l = arr; - v.obj = arr; + value.l = arr; } break; case Variant::PACKED_BYTE_ARRAY: { @@ -247,8 +238,7 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jbyteArray arr = env->NewByteArray(array.size()); const uint8_t *r = array.ptr(); env->SetByteArrayRegion(arr, 0, array.size(), reinterpret_cast(r)); - v.val.l = arr; - v.obj = arr; + value.l = arr; } break; case Variant::PACKED_FLOAT32_ARRAY: { @@ -256,8 +246,7 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jfloatArray arr = env->NewFloatArray(array.size()); const float *r = array.ptr(); env->SetFloatArrayRegion(arr, 0, array.size(), r); - v.val.l = arr; - v.obj = arr; + value.l = arr; } break; case Variant::PACKED_FLOAT64_ARRAY: { @@ -265,26 +254,25 @@ jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_a jdoubleArray arr = env->NewDoubleArray(array.size()); const double *r = array.ptr(); env->SetDoubleArrayRegion(arr, 0, array.size(), r); - v.val.l = arr; - v.obj = arr; + value.l = arr; } break; case Variant::OBJECT: { Ref generic_object = *p_arg; if (generic_object.is_valid()) { jobject obj = env->NewLocalRef(generic_object->get_instance()); - v.val.l = obj; - v.obj = obj; + value.l = obj; } else { - v.val.i = 0; + value.i = 0; } } break; default: { - v.val.i = 0; + value.i = 0; } break; } - return v; + value.l = env->PopLocalFrame(value.l); + return value; } String _get_class_name(JNIEnv *env, jclass cls, bool *array) { @@ -299,6 +287,7 @@ String _get_class_name(JNIEnv *env, jclass cls, bool *array) { } String name = jstring_to_string(clsName, env); env->DeleteLocalRef(clsName); + env->DeleteLocalRef(cclass); return name; } @@ -360,6 +349,7 @@ Variant _jobject_to_variant(JNIEnv *env, jobject obj, int p_depth) { jclass nclass = jni_find_class(env, "java/lang/Number"); jmethodID longValue = env->GetMethodID(nclass, "longValue", "()J"); jlong ret = env->CallLongMethod(obj, longValue); + env->DeleteLocalRef(nclass); return ret; } @@ -400,6 +390,7 @@ Variant _jobject_to_variant(JNIEnv *env, jobject obj, int p_depth) { jclass nclass = jni_find_class(env, "java/lang/Number"); jmethodID doubleValue = env->GetMethodID(nclass, "doubleValue", "()D"); double ret = env->CallDoubleMethod(obj, doubleValue); + env->DeleteLocalRef(nclass); return ret; } @@ -558,6 +549,11 @@ void setup_android_class_loader() { android_class_loader = nullptr; ERR_FAIL_MSG("Failed to find method ID for ClassLoader::loadClass."); } + + env->DeleteLocalRef(class_loader_class); + env->DeleteLocalRef(class_loader); + env->DeleteLocalRef(class_class); + env->DeleteLocalRef(known_class); } void cleanup_android_class_loader() { @@ -577,17 +573,22 @@ jclass jni_find_class(JNIEnv *p_env, const char *p_class_name) { ERR_FAIL_NULL_V(p_env, nullptr); ERR_FAIL_NULL_V(p_class_name, nullptr); + jobject class_object = nullptr; if (!android_class_loader || !load_class_method) { ERR_PRINT("Android ClassLoader is not initialized. Falling back to FindClass."); - return p_env->FindClass(p_class_name); + class_object = p_env->FindClass(p_class_name); + } else { + jstring java_class_name = p_env->NewStringUTF(p_class_name); + class_object = p_env->CallObjectMethod( + android_class_loader, + load_class_method, + java_class_name); + p_env->DeleteLocalRef(java_class_name); + } + if (p_env->ExceptionCheck()) { + p_env->ExceptionDescribe(); + p_env->ExceptionClear(); } - - jstring java_class_name = p_env->NewStringUTF(p_class_name); - jobject class_object = p_env->CallObjectMethod( - android_class_loader, - load_class_method, - java_class_name); - p_env->DeleteLocalRef(java_class_name); ERR_FAIL_NULL_V_MSG(class_object, nullptr, vformat("Failed to find Java class: \"%s\".", p_class_name)); return static_cast(class_object); } diff --git a/platform/android/jni_utils.h b/platform/android/jni_utils.h index 803efe09288..bbb8c1d2dd5 100644 --- a/platform/android/jni_utils.h +++ b/platform/android/jni_utils.h @@ -38,13 +38,7 @@ #include -struct jvalret { - jobject obj; - jvalue val; - jvalret() { obj = nullptr; } -}; - -jvalret _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_arg, bool force_jobject = false, int p_depth = 0); +jvalue _variant_to_jvalue(JNIEnv *env, Variant::Type p_type, const Variant *p_arg, bool force_jobject = false, int p_depth = 0); String _get_class_name(JNIEnv *env, jclass cls, bool *array); diff --git a/platform/android/variant/callable_jni.cpp b/platform/android/variant/callable_jni.cpp index cab27fbebd9..646fd8e30a8 100644 --- a/platform/android/variant/callable_jni.cpp +++ b/platform/android/variant/callable_jni.cpp @@ -91,8 +91,8 @@ JNIEXPORT jobject JNICALL Java_org_godotengine_godot_variant_Callable_nativeCall Callable::CallError err; Variant result; callable.callp(argptrs, count, result, err); - jvalret jresult = _variant_to_jvalue(p_env, result.get_type(), &result, true); - ret = jresult.obj; + jvalue jresult = _variant_to_jvalue(p_env, result.get_type(), &result, true); + ret = jresult.l; } // Manually invoke the destructor to decrease the reference counts for the variant arguments. @@ -107,8 +107,8 @@ JNIEXPORT jobject JNICALL Java_org_godotengine_godot_variant_Callable_nativeCall Callable callable = _generate_callable(p_env, p_object_id, p_method_name, p_parameters); if (callable.is_valid()) { Variant result = callable.call(); - jvalret jresult = _variant_to_jvalue(p_env, result.get_type(), &result, true); - return jresult.obj; + jvalue jresult = _variant_to_jvalue(p_env, result.get_type(), &result, true); + return jresult.l; } else { return nullptr; }