asgardius/virtualx-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #90710 from m4gr3d/fix_jstring_leaks

Browse source 
Fix leakage of JNI object references
This commit is contained in:
Rémi Verschelde 2024-04-22 12:52:16 +02:00
commit 8c474ddd49
No known key found for this signature in database
GPG key ID: C3336907360768E1
20 changed files with 139 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
modules/openxr/extensions/platform/openxr_android_extension.cpp
View file

@ -36,7 +36,6 @@
#include "os_android.h"
#include "thread_jandroid.h"
#include <jni.h>
#include <openxr/openxr.h>
#include <openxr/openxr_platform.h>
@ -48,6 +47,12 @@ OpenXRAndroidExtension *OpenXRAndroidExtension::get_singleton() {
OpenXRAndroidExtension::OpenXRAndroidExtension() {
singleton = this;
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->GetJavaVM(&vm);
activity_object = env->NewGlobalRef(static_cast<OS_Android *>(OS::get_singleton())->get_godot_java()->get_activity());
}
HashMap<String, bool *> OpenXRAndroidExtension::get_requested_extensions() {
@ -66,11 +71,6 @@ void OpenXRAndroidExtension::on_before_instance_created() {
}
loader_init_extension_available = true;
JNIEnv *env = get_jni_env();
JavaVM *vm;
env->GetJavaVM(&vm);
jobject activity_object = env->NewGlobalRef(static_cast<OS_Android *>(OS::get_singleton())->get_godot_java()->get_activity());
XrLoaderInitInfoAndroidKHR loader_init_info_android = {
.type = XR_TYPE_LOADER_INIT_INFO_ANDROID_KHR,
.next = nullptr,
@ -93,11 +93,6 @@ void *OpenXRAndroidExtension::set_instance_create_info_and_get_next_pointer(void
return nullptr;
}
JNIEnv *env = get_jni_env();
JavaVM *vm;
env->GetJavaVM(&vm);
jobject activity_object = env->NewGlobalRef(static_cast<OS_Android *>(OS::get_singleton())->get_godot_java()->get_activity());
instance_create_info = {
.type = XR_TYPE_INSTANCE_CREATE_INFO_ANDROID_KHR,
.next = p_next_pointer,
@ -109,4 +104,9 @@ void *OpenXRAndroidExtension::set_instance_create_info_and_get_next_pointer(void
OpenXRAndroidExtension::~OpenXRAndroidExtension() {
singleton = nullptr;
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(activity_object);
}

4
modules/openxr/extensions/platform/openxr_android_extension.h
View file

@ -34,6 +34,8 @@
#include "../../util.h"
#include "../openxr_extension_wrapper.h"
#include <jni.h>
class OpenXRAndroidExtension : public OpenXRExtensionWrapper {
public:
static OpenXRAndroidExtension *get_singleton();
@ -49,6 +51,8 @@ public:
private:
static OpenXRAndroidExtension *singleton;
JavaVM *vm;
jobject activity_object;
bool loader_init_extension_available = false;
bool create_instance_extension_available = false;

3
platform/android/api/java_class_wrapper.h
View file

@ -209,8 +209,6 @@ class JavaClassWrapper : public Object {
#ifdef ANDROID_ENABLED
RBMap<String, Ref<JavaClass>> class_cache;
friend class JavaClass;
jclass activityClass;
jmethodID findClass;
jmethodID getDeclaredMethods;
jmethodID getFields;
jmethodID getParameterTypes;
@ -229,7 +227,6 @@ class JavaClassWrapper : public Object {
jmethodID Long_longValue;
jmethodID Float_floatValue;
jmethodID Double_doubleValue;
jobject classLoader;
bool _get_type_sig(JNIEnv *env, jobject obj, uint32_t &sig, String &strsig);
#endif

11
platform/android/api/jni_singleton.h
View file

@ -239,6 +239,17 @@ public:
JNISingleton() {
#ifdef ANDROID_ENABLED
instance = nullptr;
#endif
}
~JNISingleton() {
#ifdef ANDROID_ENABLED
if (instance) {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(instance);
}
#endif
}
};

8
platform/android/dir_access_jandroid.cpp
View file

@ -321,6 +321,14 @@ void DirAccessJAndroid::setup(jobject p_dir_access_handler) {
_current_is_hidden = env->GetMethodID(cls, "isCurrentHidden", "(II)Z");
}
void DirAccessJAndroid::terminate() {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(cls);
env->DeleteGlobalRef(dir_access_handler);
}
DirAccessJAndroid::DirAccessJAndroid() {
}

1
platform/android/dir_access_jandroid.h
View file

@ -89,6 +89,7 @@ public:
virtual uint64_t get_space_left() override;
static void setup(jobject p_dir_access_handler);
static void terminate();
DirAccessJAndroid();
~DirAccessJAndroid();

17
platform/android/file_access_android.cpp
View file

@ -31,8 +31,12 @@
#include "file_access_android.h"
#include "core/string/print_string.h"
#include "thread_jandroid.h"
#include <android/asset_manager_jni.h>
AAssetManager *FileAccessAndroid::asset_manager = nullptr;
jobject FileAccessAndroid::j_asset_manager = nullptr;
String FileAccessAndroid::get_path() const {
return path_src;
@ -257,3 +261,16 @@ void FileAccessAndroid::close() {
FileAccessAndroid::~FileAccessAndroid() {
_close();
}
void FileAccessAndroid::setup(jobject p_asset_manager) {
JNIEnv *env = get_jni_env();
j_asset_manager = env->NewGlobalRef(p_asset_manager);
asset_manager = AAssetManager_fromJava(env, j_asset_manager);
}
void FileAccessAndroid::terminate() {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(j_asset_manager);
}

10
platform/android/file_access_android.h
View file

@ -35,9 +35,13 @@
#include <android/asset_manager.h>
#include <android/log.h>
#include <jni.h>
#include <stdio.h>
class FileAccessAndroid : public FileAccess {
static AAssetManager *asset_manager;
static jobject j_asset_manager;
mutable AAsset *asset = nullptr;
mutable uint64_t len = 0;
mutable uint64_t pos = 0;
@ -48,8 +52,6 @@ class FileAccessAndroid : public FileAccess {
void _close();
public:
static AAssetManager *asset_manager;
virtual Error open_internal(const String &p_path, int p_mode_flags) override; // open a file
virtual bool is_open() const override; // true when file is open
@ -93,6 +95,10 @@ public:
virtual void close() override;
static void setup(jobject p_asset_manager);
static void terminate();
~FileAccessAndroid();
};

8
platform/android/file_access_filesystem_jandroid.cpp
View file

@ -408,6 +408,14 @@ void FileAccessFilesystemJAndroid::setup(jobject p_file_access_handler) {
_file_resize = env->GetMethodID(cls, "fileResize", "(IJ)I");
}
void FileAccessFilesystemJAndroid::terminate() {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(cls);
env->DeleteGlobalRef(file_access_handler);
}
void FileAccessFilesystemJAndroid::close() {
if (is_open()) {
_close();

1
platform/android/file_access_filesystem_jandroid.h
View file

@ -97,6 +97,7 @@ public:
virtual bool file_exists(const String &p_path) override; ///< return true if a file exists
static void setup(jobject p_file_access_handler);
static void terminate();
virtual uint64_t _get_modified_time(const String &p_file) override;
virtual BitField<FileAccess::UnixPermissionFlags> _get_unix_permissions(const String &p_file) override { return 0; }

18
platform/android/java_class_wrapper.cpp
View file

@ -1157,50 +1157,54 @@ JavaClassWrapper::JavaClassWrapper(jobject p_activity) {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
jclass activity = env->FindClass("android/app/Activity");
jmethodID getClassLoader = env->GetMethodID(activity, "getClassLoader", "()Ljava/lang/ClassLoader;");
classLoader = env->CallObjectMethod(p_activity, getClassLoader);
classLoader = (jclass)env->NewGlobalRef(classLoader);
jclass classLoaderClass = env->FindClass("java/lang/ClassLoader");
findClass = env->GetMethodID(classLoaderClass, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;");
jclass bclass = env->FindClass("java/lang/Class");
getDeclaredMethods = env->GetMethodID(bclass, "getDeclaredMethods", "()[Ljava/lang/reflect/Method;");
getFields = env->GetMethodID(bclass, "getFields", "()[Ljava/lang/reflect/Field;");
Class_getName = env->GetMethodID(bclass, "getName", "()Ljava/lang/String;");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/reflect/Method");
getParameterTypes = env->GetMethodID(bclass, "getParameterTypes", "()[Ljava/lang/Class;");
getReturnType = env->GetMethodID(bclass, "getReturnType", "()Ljava/lang/Class;");
getName = env->GetMethodID(bclass, "getName", "()Ljava/lang/String;");
getModifiers = env->GetMethodID(bclass, "getModifiers", "()I");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/reflect/Field");
Field_getName = env->GetMethodID(bclass, "getName", "()Ljava/lang/String;");
Field_getModifiers = env->GetMethodID(bclass, "getModifiers", "()I");
Field_get = env->GetMethodID(bclass, "get", "(Ljava/lang/Object;)Ljava/lang/Object;");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Boolean");
Boolean_booleanValue = env->GetMethodID(bclass, "booleanValue", "()Z");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Byte");
Byte_byteValue = env->GetMethodID(bclass, "byteValue", "()B");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Character");
Character_characterValue = env->GetMethodID(bclass, "charValue", "()C");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Short");
Short_shortValue = env->GetMethodID(bclass, "shortValue", "()S");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Integer");
Integer_integerValue = env->GetMethodID(bclass, "intValue", "()I");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Long");
Long_longValue = env->GetMethodID(bclass, "longValue", "()J");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Float");
Float_floatValue = env->GetMethodID(bclass, "floatValue", "()F");
env->DeleteLocalRef(bclass);
bclass = env->FindClass("java/lang/Double");
Double_doubleValue = env->GetMethodID(bclass, "doubleValue", "()D");
env->DeleteLocalRef(bclass);
}

11
platform/android/java_godot_io_wrapper.cpp
View file

@ -70,7 +70,11 @@ GodotIOJavaWrapper::GodotIOJavaWrapper(JNIEnv *p_env, jobject p_godot_io_instanc
}
GodotIOJavaWrapper::~GodotIOJavaWrapper() {
// nothing to do here for now
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(cls);
env->DeleteGlobalRef(godot_io_instance);
}
jobject GodotIOJavaWrapper::get_instance() {
@ -82,7 +86,9 @@ Error GodotIOJavaWrapper::open_uri(const String &p_uri) {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL_V(env, ERR_UNAVAILABLE);
jstring jStr = env->NewStringUTF(p_uri.utf8().get_data());
return env->CallIntMethod(godot_io_instance, _open_URI, jStr) ? ERR_CANT_OPEN : OK;
Error result = env->CallIntMethod(godot_io_instance, _open_URI, jStr) ? ERR_CANT_OPEN : OK;
env->DeleteLocalRef(jStr);
return result;
} else {
return ERR_UNAVAILABLE;
}
@ -220,6 +226,7 @@ void GodotIOJavaWrapper::show_vk(const String &p_existing, int p_type, int p_max
ERR_FAIL_NULL(env);
jstring jStr = env->NewStringUTF(p_existing.utf8().get_data());
env->CallVoidMethod(godot_io_instance, _show_keyboard, jStr, p_type, p_max_input_length, p_cursor_start, p_cursor_end);
env->DeleteLocalRef(jStr);
}
}

12
platform/android/java_godot_lib_jni.cpp
View file

@ -95,6 +95,13 @@ static void _terminate(JNIEnv *env, bool p_restart = false) {
if (godot_io_java) {
delete godot_io_java;
}
TTS_Android::terminate();
FileAccessAndroid::terminate();
DirAccessJAndroid::terminate();
FileAccessFilesystemJAndroid::terminate();
NetSocketAndroid::terminate();
if (godot_java) {
if (!restart_on_cleanup) {
if (p_restart) {
@ -125,10 +132,7 @@ JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_GodotLib_initialize(JNIEnv
init_thread_jandroid(jvm, env);
jobject amgr = env->NewGlobalRef(p_asset_manager);
FileAccessAndroid::asset_manager = AAssetManager_fromJava(env, amgr);
FileAccessAndroid::setup(p_asset_manager);
DirAccessJAndroid::setup(p_directory_access_handler);
FileAccessFilesystemJAndroid::setup(p_file_access_handler);
NetSocketAndroid::setup(p_net_utils);

1
platform/android/java_godot_view_wrapper.cpp
View file

@ -95,6 +95,7 @@ void GodotJavaViewWrapper::configure_pointer_icon(int pointer_type, const String
jstring jImagePath = env->NewStringUTF(image_path.utf8().get_data());
env->CallVoidMethod(_godot_view, _configure_pointer_icon, pointer_type, jImagePath, p_hotspot.x, p_hotspot.y);
env->DeleteLocalRef(jImagePath);
}
}

20
platform/android/java_godot_wrapper.cpp
View file

@ -172,6 +172,8 @@ void GodotJavaWrapper::alert(const String &p_message, const String &p_title) {
jstring jStrMessage = env->NewStringUTF(p_message.utf8().get_data());
jstring jStrTitle = env->NewStringUTF(p_title.utf8().get_data());
env->CallVoidMethod(godot_instance, _alert, jStrMessage, jStrTitle);
env->DeleteLocalRef(jStrMessage);
env->DeleteLocalRef(jStrTitle);
}
}
@ -231,6 +233,7 @@ void GodotJavaWrapper::set_clipboard(const String &p_text) {
ERR_FAIL_NULL(env);
jstring jStr = env->NewStringUTF(p_text.utf8().get_data());
env->CallVoidMethod(godot_instance, _set_clipboard, jStr);
env->DeleteLocalRef(jStr);
}
}
@ -253,7 +256,9 @@ bool GodotJavaWrapper::request_permission(const String &p_name) {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL_V(env, false);
jstring jStrName = env->NewStringUTF(p_name.utf8().get_data());
return env->CallBooleanMethod(godot_instance, _request_permission, jStrName);
bool result = env->CallBooleanMethod(godot_instance, _request_permission, jStrName);
env->DeleteLocalRef(jStrName);
return result;
} else {
return false;
}
@ -340,7 +345,9 @@ int GodotJavaWrapper::create_new_godot_instance(const List<String> &args) {
ERR_FAIL_NULL_V(env, 0);
jobjectArray jargs = env->NewObjectArray(args.size(), env->FindClass("java/lang/String"), env->NewStringUTF(""));
for (int i = 0; i < args.size(); i++) {
env->SetObjectArrayElement(jargs, i, env->NewStringUTF(args[i].utf8().get_data()));
jstring j_arg = env->NewStringUTF(args[i].utf8().get_data());
env->SetObjectArrayElement(jargs, i, j_arg);
env->DeleteLocalRef(j_arg);
}
return env->CallIntMethod(godot_instance, _create_new_godot_instance, jargs);
} else {
@ -355,6 +362,8 @@ void GodotJavaWrapper::begin_benchmark_measure(const String &p_context, const St
jstring j_context = env->NewStringUTF(p_context.utf8().get_data());
jstring j_label = env->NewStringUTF(p_label.utf8().get_data());
env->CallVoidMethod(godot_instance, _begin_benchmark_measure, j_context, j_label);
env->DeleteLocalRef(j_context);
env->DeleteLocalRef(j_label);
}
}
@ -365,6 +374,8 @@ void GodotJavaWrapper::end_benchmark_measure(const String &p_context, const Stri
jstring j_context = env->NewStringUTF(p_context.utf8().get_data());
jstring j_label = env->NewStringUTF(p_label.utf8().get_data());
env->CallVoidMethod(godot_instance, _end_benchmark_measure, j_context, j_label);
env->DeleteLocalRef(j_context);
env->DeleteLocalRef(j_label);
}
}
@ -374,6 +385,7 @@ void GodotJavaWrapper::dump_benchmark(const String &benchmark_file) {
ERR_FAIL_NULL(env);
jstring j_benchmark_file = env->NewStringUTF(benchmark_file.utf8().get_data());
env->CallVoidMethod(godot_instance, _dump_benchmark, j_benchmark_file);
env->DeleteLocalRef(j_benchmark_file);
}
}
@ -383,7 +395,9 @@ bool GodotJavaWrapper::has_feature(const String &p_feature) const {
ERR_FAIL_NULL_V(env, false);
jstring j_feature = env->NewStringUTF(p_feature.utf8().get_data());
return env->CallBooleanMethod(godot_instance, _has_feature, j_feature);
bool result = env->CallBooleanMethod(godot_instance, _has_feature, j_feature);
env->DeleteLocalRef(j_feature);
return result;
} else {
return false;
}

8
platform/android/net_socket_android.cpp
View file

@ -49,6 +49,14 @@ void NetSocketAndroid::setup(jobject p_net_utils) {
_multicast_lock_release = env->GetMethodID(cls, "multicastLockRelease", "()V");
}
void NetSocketAndroid::terminate() {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(cls);
env->DeleteGlobalRef(net_utils);
}
void NetSocketAndroid::multicast_lock_acquire() {
if (_multicast_lock_acquire) {
JNIEnv *env = get_jni_env();

1
platform/android/net_socket_android.h
View file

@ -63,6 +63,7 @@ protected:
public:
static void make_default();
static void setup(jobject p_net_utils);
static void terminate();
virtual void close();

4
platform/android/os_android.cpp
View file

@ -776,6 +776,10 @@ void OS_Android::benchmark_dump() {
}
bool OS_Android::_check_internal_feature_support(const String &p_feature) {
if (p_feature == "macos" || p_feature == "web_ios" || p_feature == "web_macos" || p_feature == "windows") {
return false;
}
if (p_feature == "system_fonts") {
return true;
}

10
platform/android/tts_android.cpp
View file

@ -77,6 +77,14 @@ void TTS_Android::setup(jobject p_tts) {
}
}
void TTS_Android::terminate() {
JNIEnv *env = get_jni_env();
ERR_FAIL_NULL(env);
env->DeleteGlobalRef(cls);
env->DeleteGlobalRef(tts);
}
void TTS_Android::_java_utterance_callback(int p_event, int p_id, int p_pos) {
ERR_FAIL_COND_MSG(!initialized, "Enable the \"audio/general/text_to_speech\" project setting to use text-to-speech.");
if (ids.has(p_id)) {
@ -170,6 +178,8 @@ void TTS_Android::speak(const String &p_text, const String &p_voice, int p_volum
jstring jStrT = env->NewStringUTF(p_text.utf8().get_data());
jstring jStrV = env->NewStringUTF(p_voice.utf8().get_data());
env->CallVoidMethod(tts, _speak, jStrT, jStrV, CLAMP(p_volume, 0, 100), CLAMP(p_pitch, 0.f, 2.f), CLAMP(p_rate, 0.1f, 10.f), p_utterance_id, p_interrupt);
env->DeleteLocalRef(jStrT);
env->DeleteLocalRef(jStrV);
}
}

1
platform/android/tts_android.h
View file

@ -57,6 +57,7 @@ class TTS_Android {
public:
static void setup(jobject p_tts);
static void terminate();
static void _java_utterance_callback(int p_event, int p_id, int p_pos);
static bool is_speaking();