Remove binding from both Object and RefCounted when closing (#680)

utopia-rise · Sep 1, 2024 · d032cc1 · d032cc1
1 parent 757a39b
commit d032cc1
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 16 deletions.
diff --git a/kt/godot-library/src/main/kotlin/godot/core/memory/MemoryManager.kt b/kt/godot-library/src/main/kotlin/godot/core/memory/MemoryManager.kt
@@ -258,9 +258,9 @@ internal object MemoryManager {
             callback.invoke()
         }
 
-        // Get through all remaining [RefCounted] instances and decrement their pointers.
-        for (ref in ObjectDB.filterNotNull().filter { it.objectID.isReference }) {
-            decrementRefCounter(ref.objectID.id)
+        // Get through all remaining [Objects] instances and remove their bindings
+        for (ref in ObjectDB.filterNotNull()) {
+            releaseBinding(ref.objectID.id)
         }
         ObjectDB.fill(null)
 
@@ -275,7 +275,7 @@ internal object MemoryManager {
     external fun getSingleton(classIndex: Int)
     external fun createNativeObject(classIndex: Int, instance: KtObject, scriptIndex: Int)
     external fun checkInstance(ptr: VoidPtr, instanceId: Long): Boolean
-    external fun decrementRefCounter(instanceId: Long)
+    external fun releaseBinding(instanceId: Long)
     external fun freeObject(rawPtr: VoidPtr)
     external fun unrefNativeCoreType(ptr: VoidPtr, variantType: Int): Boolean
     external fun querySync()

diff --git a/...ns/godot-gradle-plugin/src/main/resources/godot/gradle/godot-kotlin-graal-jni-config.json b/...ns/godot-gradle-plugin/src/main/resources/godot/gradle/godot-kotlin-graal-jni-config.json
@@ -49,7 +49,7 @@
     ],
     "methods" : [
       { "name" : "icall", "parameterTypes" : ["long", "long", "int"] },
-      { "name" : "getBuffer", "parameterTypes" : [] },
+      { "name" : "getBuffer", "parameterTypes" : [] }
 
     ]
   },
@@ -80,7 +80,7 @@
       { "name" : "cleanUp","parameterTypes" : [] },
       { "name" : "removeScriptInstance", "parameterTypes" : ["long"] },
       { "name" : "checkInstance", "parameterTypes" : ["long", "long"] },
-      { "name" : "decrementRefCounter", "parameterTypes" : ["long"] },
+      { "name" : "releaseBinding", "parameterTypes" : ["long"] },
       { "name" : "unrefNativeCoreType", "parameterTypes" : ["long", "int"] },
       { "name" : "querySync", "parameterTypes" : [] },
       { "name" : "createNativeObject", "parameterTypes" : ["int", "godot.core.KtObject", "int"] },

diff --git a/src/binding/kotlin_binding_manager.cpp b/src/binding/kotlin_binding_manager.cpp
@@ -57,8 +57,6 @@ KotlinBinding* KotlinBindingManager::get_instance_binding(Object* p_object) {
     return binding;
 }
 
-void KotlinBindingManager::decrement_counter(RefCounted* p_ref) {
+void KotlinBindingManager::free_binding(Object* p_ref) {
     p_ref->free_instance_binding(&GDKotlin::get_instance());
-
-    if (p_ref->unreference()) { memdelete(p_ref); }
 }
diff --git a/src/binding/kotlin_binding_manager.h b/src/binding/kotlin_binding_manager.h
@@ -30,7 +30,7 @@ class KotlinBindingManager {
     // Doesn't set the KtObject as it doesn't exist yet, bind_object has be used later.
     static KotlinBinding* get_instance_binding(Object* p_object);
 
-    static void decrement_counter(RefCounted* p_ref);
+    static void free_binding(Object* p_ref);
 };
 
 #endif// GODOT_JVM_KOTLIN_BINDING_MANAGER_H
diff --git a/src/jvm_wrapper/memory/memory_manager.cpp b/src/jvm_wrapper/memory/memory_manager.cpp
@@ -11,9 +11,15 @@ bool MemoryManager::check_instance(JNIEnv* p_raw_env, jobject p_instance, jlong
     return instance == ObjectDB::get_instance(static_cast<ObjectID>(static_cast<uint64_t>(instance_id)));
 }
 
-void MemoryManager::decrement_ref_counter(JNIEnv* p_raw_env, jobject p_instance, jlong instance_id) {
+void MemoryManager::release_binding(JNIEnv* p_raw_env, jobject p_instance, jlong instance_id) {
     Object* obj = ObjectDB::get_instance(static_cast<ObjectID>(static_cast<uint64_t>(instance_id)));
-    KotlinBindingManager::decrement_counter(reinterpret_cast<RefCounted*>(obj));
+    if (obj == nullptr) { return; }
+
+    KotlinBindingManager::free_binding(obj);
+    if (obj->is_ref_counted()) {
+        RefCounted* ref = reinterpret_cast<RefCounted*>(obj);
+        if (ref->unreference()) { memdelete(ref); }
+    }
 }
 
 bool MemoryManager::unref_native_core_type(JNIEnv* p_raw_env, jobject p_instance, jlong p_raw_ptr, jint var_type) {
@@ -175,8 +181,9 @@ bool MemoryManager::sync_memory(jni::Env& p_env) {
     refs_to_decrement.delete_local_ref(p_env);
 
     for (uint64_t id : vec) {
-        Object* obj = ObjectDB::get_instance(static_cast<ObjectID>(id));
-        KotlinBindingManager::decrement_counter(reinterpret_cast<RefCounted*>(obj));
+        RefCounted* ref = reinterpret_cast<RefCounted*>(ObjectDB::get_instance(static_cast<ObjectID>(id)));
+        KotlinBindingManager::free_binding(ref);
+        if (ref->unreference()) { memdelete(ref); }
     }
 
     return active;

diff --git a/src/jvm_wrapper/memory/memory_manager.h b/src/jvm_wrapper/memory/memory_manager.h
@@ -22,12 +22,12 @@ JVM_SINGLETON_WRAPPER(MemoryManager, "godot.core.memory.MemoryManager") {
         INIT_JNI_METHOD(CLEAN_UP, "cleanUp", "()V")
         INIT_JNI_METHOD(REMOVE_SCRIPT, "removeScriptInstance", "(J)V")
         INIT_NATIVE_METHOD("checkInstance", "(JJ)Z", MemoryManager::check_instance)
-        INIT_NATIVE_METHOD("decrementRefCounter", "(J)V", MemoryManager::decrement_ref_counter)
         INIT_NATIVE_METHOD("unrefNativeCoreType", "(JI)Z", MemoryManager::unref_native_core_type)
         INIT_NATIVE_METHOD("querySync", "()V", MemoryManager::query_sync)
         INIT_NATIVE_METHOD("createNativeObject", "(ILgodot/core/KtObject;I)V", MemoryManager::create_native_object)
         INIT_NATIVE_METHOD("getSingleton", "(I)V", MemoryManager::get_singleton)
         INIT_NATIVE_METHOD("freeObject", "(J)V", MemoryManager::free_object)
+        INIT_NATIVE_METHOD("releaseBinding", "(J)V", MemoryManager::release_binding)
       )
 
     Mutex dead_objects_mutex;
@@ -36,12 +36,12 @@ JVM_SINGLETON_WRAPPER(MemoryManager, "godot.core.memory.MemoryManager") {
     HashSet<JvmInstance*> to_demote_objects;
 
     static bool check_instance(JNIEnv* p_raw_env, jobject p_instance, jlong p_raw_ptr, jlong instance_id);
-    static void decrement_ref_counter(JNIEnv* p_raw_env, jobject p_instance, jlong instance_id);
     static bool unref_native_core_type(JNIEnv* p_raw_env, jobject p_instance, jlong p_raw_ptr, jint var_type);
     static void query_sync(JNIEnv* p_raw_env, jobject p_instance);
     static void create_native_object(JNIEnv* p_raw_env, jobject instance, jint p_class_index, jobject p_object, jint p_script_index);
     static void get_singleton(JNIEnv* p_raw_env, jobject p_instance, jint p_class_index);
     static void free_object(JNIEnv* p_raw_env, jobject p_instance, jlong p_raw_ptr);
+    static void release_binding(JNIEnv* p_raw_env, jobject p_instance, jlong instance_id);
 
 public:
     void remove_script_instance(jni::Env& p_env, uint64_t id);