Improve flow of jni::Env variables (#597)

src/binding/kotlin_binding.cpp

@@ -13,7 +13,8 @@ void KotlinBinding::refcount_incremented_unsafe() {
     if (refcount > 1 && kt_binding->is_ref_weak()) {
         // The JVM holds a reference to that object already, if the counter is greater than 1, it means the native side holds a reference as well.
         // The reference is changed to a strong one so the JVM instance is not collected if it is not referenced anymore on the JVM side.
-        kt_binding->swap_to_strong_unsafe();
+        jni::Env env {jni::Jvm::current_env()};
+        kt_binding->swap_to_strong_unsafe(env);
     }
 }
 
@@ -28,15 +29,18 @@ bool KotlinBinding::refcount_decremented_unsafe() {
     if (refcount == 1 && !kt_binding->is_ref_weak()) {
         // The JVM holds a reference to that object already, if the counter is equal to 1, it means the JVM is the only side with a reference to the object.
         // The reference is changed to a weak one so the JVM instance can be collected if it is not referenced anymore on the JVM side.
-        kt_binding->swap_to_weak_unsafe();
+        jni::Env env {jni::Jvm::current_env()};
+        kt_binding->swap_to_weak_unsafe(env);
     }
     // Return true when the counter is 0, it means that the JVM and the native side are no longer using the reference, so it can be safely deleted.
     return refcount == 0;
 }
 
 void KotlinBinding::set_kt_binding(jni::JObject j_object) {
     JVM_CRASH_COND_MSG(kt_binding, "Trying to set a KtBinding on an already bound Object");
-    kt_binding = memnew(KtBinding(j_object));
+
+    jni::Env env {jni::Jvm::current_env()};
+    kt_binding = memnew(KtBinding(env, j_object));
 
     if (!owner->is_ref_counted()) {
         return;
@@ -48,7 +52,7 @@ void KotlinBinding::set_kt_binding(jni::JObject j_object) {
     if (refcount == 1 && !kt_binding->is_ref_weak()) {
         // The JVM holds a reference to that object already, if the counter is equal to 1, it means the JVM is the only side with a reference to the object.
         // The reference is changed to a weak one so the JVM instance can be collected if it is not referenced anymore on the JVM side.
-        kt_binding->swap_to_weak_unsafe();
+        kt_binding->swap_to_weak_unsafe(env);
     }
     status = BindingStatus::BOUND;
 }

src/gd_kotlin.cpp

@@ -203,11 +203,11 @@ void GDKotlin::init() {
     }
 #endif
 
-    JniLifecycleManager::initialize_jni_classes();
+    JniLifecycleManager::initialize_jni_classes(env);
 
     int max_string_size {configuration.get_max_string_size()};
     if (max_string_size != LongStringQueue::max_string_size) {
-        LongStringQueue::get_instance().set_string_max_size(max_string_size);
+        LongStringQueue::get_instance().set_string_max_size(env, max_string_size);
     }
 
     if (is_gc_activated) {
@@ -217,18 +217,18 @@ void GDKotlin::init() {
 #endif
         }
 
-        MemoryManager::get_instance().start(is_gc_force_mode);
+        MemoryManager::get_instance().start(env, is_gc_force_mode);
 #ifdef DEBUG_ENABLED
         LOG_VERBOSE("GC thread started.");
 #endif
         is_gc_started = true;
     }
 
     if (!should_display_leaked_jvm_instances_on_close) {
-        MemoryManager::get_instance().setDisplayLeaks(false);
+        MemoryManager::get_instance().setDisplayLeaks(env, false);
     }
 
-    bootstrap = Bootstrap::create_instance();
+    bootstrap = Bootstrap::create_instance(env);
 
 #ifdef TOOLS_ENABLED
     String jar_path {project_settings->globalize_path("res://build/libs/")};
@@ -268,15 +268,15 @@ void GDKotlin::finish() {
     TypeManager::get_instance().clear();
 
     if (is_gc_started) {
-        MemoryManager::get_instance().close();
+        MemoryManager::get_instance().close(env);
 
-        while (!MemoryManager::get_instance().is_closed()) {
+        while (!MemoryManager::get_instance().is_closed(env)) {
             OS::get_singleton()->delay_usec(600000);
         }
 #ifdef DEBUG_ENABLED
         LOG_VERBOSE("JVM GC thread was closed");
 #endif
-        MemoryManager::get_instance().clean_up();
+        MemoryManager::get_instance().clean_up(env);
     }
 
     JniLifecycleManager::destroy_jni_classes();
@@ -290,10 +290,12 @@ void GDKotlin::register_classes(jni::Env& p_env, jni::JObjectArray p_classes) {
 #ifdef DEV_ENABLED
     LOG_INFO("Loading classes ...");
 #endif
+
+    jni::Env env {jni::Jvm::current_env()};
     Vector<KtClass*> classes;
     for (auto i = 0; i < p_classes.length(p_env); i++) {
-        KtClass* kt_class = new KtClass(p_classes.get(p_env, i));
-        kt_class->fetch_members();
+        KtClass* kt_class = new KtClass(env, p_classes.get(p_env, i));
+        kt_class->fetch_members(env);
         classes.append(kt_class);
 #ifdef DEV_ENABLED
         LOG_VERBOSE(vformat("Loaded class %s : %s", kt_class->registered_class_name, kt_class->base_godot_class));

src/jni/env.h

@@ -19,7 +19,7 @@ namespace jni {
     class Env {
     public:
         explicit Env(JNIEnv*);
-        // todo delete copy ctor and assignment?
+
         Env(const Env&) = default;
         Env& operator=(const Env&) = default;
 

src/jni/jvm.h

@@ -36,7 +36,6 @@ namespace jni {
     private:
         Jvm() = default;
         static JavaVM* vm;
-        static Env* env;
         static jint version;
         static Type vm_type;
 

src/jni/platforms/jvm_android.cpp

@@ -6,7 +6,6 @@
 
 namespace jni {
     JavaVM* Jvm::vm = nullptr;
-    Env* Jvm::env = nullptr;
     jint Jvm::version = 0;
     Jvm::Type Jvm::vm_type {Jvm::ART};
 

src/jni/platforms/jvm_default.cpp

@@ -11,10 +11,10 @@
 #include "../jvm_loader.h"
 #include "jni/jvm.h"
 
-
 namespace jni {
+    static thread_local Env* env = nullptr;
+
     JavaVM* Jvm::vm = nullptr;
-    Env* Jvm::env = nullptr;
     jint Jvm::version = 0;
     Jvm::Type Jvm::vm_type {Jvm::JVM};
 
@@ -63,6 +63,7 @@ namespace jni {
 
         delete[] options;
         JVM_CRASH_COND_MSG(result != JNI_OK, "Failed to create a new vm!");
+        env = new Env(jni_env);
         return java_vm;
     }
 
@@ -79,22 +80,25 @@ namespace jni {
         JNIEnv* r_env;
         auto result = vm->AttachCurrentThread((void**) &r_env, nullptr);
         JVM_CRASH_COND_MSG(result != JNI_OK, "Failed to attach vm to current thread!");
-        Jvm::env = new Env(r_env);
+        env = new Env(r_env);
         return Env(r_env);
     }
 
     void Jvm::detach() {
         auto result = vm->DetachCurrentThread();
         JVM_CRASH_COND_MSG(result != JNI_OK, "Failed to detach vm to current thread!");
-        delete Jvm::env;
-        Jvm::env = nullptr;
+        delete env;
+        env = nullptr;
     }
 
     Env Jvm::current_env() {
-        JNIEnv* r_env;
-        auto result = vm->GetEnv((void**) &r_env, version);
-        JVM_CRASH_COND_MSG(result == JNI_EDETACHED, "Current thread is not attached!");
-        return Env(r_env);
+        if (unlikely(!env)) {
+            JNIEnv* r_env;
+            auto result = vm->GetEnv((void**) &r_env, version);
+            JVM_CRASH_COND_MSG(result == JNI_EDETACHED, "Current thread is not attached!");
+            env = new Env(r_env);
+        }
+        return *env;
     }
 
     Jvm::Type Jvm::get_type() {

src/jni_lifecycle_manager.cpp

@@ -21,44 +21,44 @@
 #include "jvm_wrapper/memory/transfer_context.h"
 #include "jvm_wrapper/registration//kt_class.h"
 
-void JniLifecycleManager::initialize_jni_classes() {
+void JniLifecycleManager::initialize_jni_classes(jni::Env& p_env) {
     // Singleton
-    TransferContext::initialize();
-    TypeManager::initialize();
-    LongStringQueue::initialize();
-    MemoryManager::initialize();
+    TransferContext::initialize(p_env);
+    TypeManager::initialize(p_env);
+    LongStringQueue::initialize(p_env);
+    MemoryManager::initialize(p_env);
 
-    bridges::GDPrintBridge::initialize();
+    bridges::GDPrintBridge::initialize(p_env);
 
-    bridges::CallableBridge::initialize();
-    bridges::DictionaryBridge::initialize();
-    bridges::RidBridge::initialize();
-    bridges::StringNameBridge::initialize();
-    bridges::NodePathBridge::initialize();
-    bridges::VariantArrayBridge::initialize();
+    bridges::CallableBridge::initialize(p_env);
+    bridges::DictionaryBridge::initialize(p_env);
+    bridges::RidBridge::initialize(p_env);
+    bridges::StringNameBridge::initialize(p_env);
+    bridges::NodePathBridge::initialize(p_env);
+    bridges::VariantArrayBridge::initialize(p_env);
 
-    bridges::PackedByteArrayBridge::initialize();
-    bridges::PackedColorArrayBridge::initialize();
-    bridges::PackedFloat32ArrayBridge::initialize();
-    bridges::PackedFloat64ArrayBridge::initialize();
-    bridges::PackedInt32IntArrayBridge::initialize();
-    bridges::PackedInt64IntArrayBridge::initialize();
-    bridges::PackedStringArrayBridge::initialize();
-    bridges::PackedVector2ArrayBridge::initialize();
-    bridges::PackedVector3ArrayBridge::initialize();
+    bridges::PackedByteArrayBridge::initialize(p_env);
+    bridges::PackedColorArrayBridge::initialize(p_env);
+    bridges::PackedFloat32ArrayBridge::initialize(p_env);
+    bridges::PackedFloat64ArrayBridge::initialize(p_env);
+    bridges::PackedInt32IntArrayBridge::initialize(p_env);
+    bridges::PackedInt64IntArrayBridge::initialize(p_env);
+    bridges::PackedStringArrayBridge::initialize(p_env);
+    bridges::PackedVector2ArrayBridge::initialize(p_env);
+    bridges::PackedVector3ArrayBridge::initialize(p_env);
 
     //Instance
-    Bootstrap::initialize_jni_binding();
-    KtObject::initialize_jni_binding();
+    Bootstrap::initialize_jni_binding(p_env);
+    KtObject::initialize_jni_binding(p_env);
 
-    KtPropertyInfo::initialize_jni_binding();
-    KtProperty::initialize_jni_binding();
-    KtConstructor::initialize_jni_binding();
-    KtSignalInfo::initialize_jni_binding();
-    KtRpcConfig::initialize_jni_binding();
-    KtFunctionInfo::initialize_jni_binding();
-    KtFunction::initialize_jni_binding();
-    KtClass::initialize_jni_binding();
+    KtPropertyInfo::initialize_jni_binding(p_env);
+    KtProperty::initialize_jni_binding(p_env);
+    KtConstructor::initialize_jni_binding(p_env);
+    KtSignalInfo::initialize_jni_binding(p_env);
+    KtRpcConfig::initialize_jni_binding(p_env);
+    KtFunctionInfo::initialize_jni_binding(p_env);
+    KtFunction::initialize_jni_binding(p_env);
+    KtClass::initialize_jni_binding(p_env);
 }
 
 void JniLifecycleManager::destroy_jni_classes() {

src/jni_lifecycle_manager.h

@@ -1,9 +1,11 @@
 #ifndef GODOT_JVM_JNI_LIFECYCLE_MANAGER_H
 #define GODOT_JVM_JNI_LIFECYCLE_MANAGER_H
 
+#include "jni/env.h"
+
 class JniLifecycleManager {
 public:
-    static void initialize_jni_classes();
+    static void initialize_jni_classes(jni::Env& p_env);
     static void destroy_jni_classes();
 };
 

src/jvm_wrapper/bootstrap.cpp

@@ -34,7 +34,7 @@ void Bootstrap::register_engine_type(JNIEnv* p_env, jobject p_this, jobjectArray
 #endif
 }
 
-Bootstrap::Bootstrap(jni::JObject p_wrapped) : JvmInstanceWrapper(p_wrapped) {}
+Bootstrap::Bootstrap(jni::Env& p_env, jni::JObject p_wrapped) : JvmInstanceWrapper(p_env, p_wrapped) {}
 
 
 void Bootstrap::init(jni::Env& p_env, bool p_is_editor, const String& p_project_path, const String& p_jar_path, const String& p_jar_file, const jni::JObject& p_class_loader) {

src/jvm_wrapper/bootstrap.h

@@ -25,7 +25,7 @@ JVM_INSTANCE_WRAPPER(Bootstrap, "godot.runtime.Bootstrap") {
       jobjectArray p_singleton_names
     );
 
-    explicit Bootstrap(jni::JObject p_wrapped);
+    Bootstrap(jni::Env& p_env, jni::JObject p_wrapped);
     ~Bootstrap() = default;
 
     void init(jni::Env& p_env, bool p_is_editor, const String& p_project_path, const String& p_jar_path, const String& p_jar_file, const jni::JObject& p_class_loader);