Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix leakage of JNI object references #90710

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Fix leakage of JNI object references #90710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions modules/openxr/extensions/platform/openxr_android_extension.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "os_android.h"
#include "thread_jandroid.h"

#include <jni.h>
#include <openxr/openxr.h>
#include <openxr/openxr_platform.h>

Expand All @@ -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() {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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 changes: 4 additions & 0 deletions modules/openxr/extensions/platform/openxr_android_extension.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
#include "../../util.h"
#include "../openxr_extension_wrapper.h"

#include <jni.h>

class OpenXRAndroidExtension : public OpenXRExtensionWrapper {
public:
static OpenXRAndroidExtension *get_singleton();
Expand All @@ -49,6 +51,8 @@ class OpenXRAndroidExtension : public OpenXRExtensionWrapper {
private:
static OpenXRAndroidExtension *singleton;

JavaVM *vm;
jobject activity_object;
bool loader_init_extension_available = false;
bool create_instance_extension_available = false;

Expand Down
3 changes: 0 additions & 3 deletions platform/android/api/java_class_wrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
11 changes: 11 additions & 0 deletions platform/android/api/jni_singleton.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,17 @@ class JNISingleton : public Object {
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
}
};
Expand Down
8 changes: 8 additions & 0 deletions platform/android/dir_access_jandroid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
}

Expand Down
1 change: 1 addition & 0 deletions platform/android/dir_access_jandroid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class DirAccessJAndroid : public DirAccessUnix {
virtual uint64_t get_space_left() override;

static void setup(jobject p_dir_access_handler);
static void terminate();

DirAccessJAndroid();
~DirAccessJAndroid();
Expand Down
17 changes: 17 additions & 0 deletions platform/android/file_access_android.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 changes: 8 additions & 2 deletions platform/android/file_access_android.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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

Expand Down Expand Up @@ -92,6 +94,10 @@ class FileAccessAndroid : public FileAccess {

virtual void close() override;

static void setup(jobject p_asset_manager);

static void terminate();

~FileAccessAndroid();
};

Expand Down
8 changes: 8 additions & 0 deletions platform/android/file_access_filesystem_jandroid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,14 @@ void FileAccessFilesystemJAndroid::setup(jobject p_file_access_handler) {
_file_last_modified = env->GetMethodID(cls, "fileLastModified", "(Ljava/lang/String;)J");
}

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();
Expand Down
1 change: 1 addition & 0 deletions platform/android/file_access_filesystem_jandroid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class FileAccessFilesystemJAndroid : public FileAccess {
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; }
Expand Down
18 changes: 11 additions & 7 deletions platform/android/java_class_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 9 additions & 2 deletions platform/android/java_godot_io_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
12 changes: 8 additions & 4 deletions platform/android/java_godot_lib_jni.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions platform/android/java_godot_view_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Loading
Loading