utopia-rise / godot-kotlin-jvm

Godot Kotlin JVM Module
MIT License
613 stars 44 forks source link

memory leaks when closing the game window in different ways #653

Closed kostaskougios closed 1 month ago

kostaskougios commented 3 months ago

I've noticed memory leaks when pressing cmd-q (on macos), less memory leaks when closing the game window and no memory leaks if I add a button that does get_tree().quit() when pressed.

I am not sure what is the issue. I have some cleanup code in a function and in all cases my cleanup function is called (I have a GD.print and can see the output). But especially cmd-q almost always results in a memory leak. I am not sure if it is a bug or if it is a problem of my code. But I would think since my cleanup() is called in all cases and no leaks happen when I close the game by pressing my button that does the get_tree().quit(), it has to do with the handlers of window-close or cmd-q events.

jsbeckr commented 3 months ago

If you start godot in a terminal it will print out which instances leaked. That helped me find an annoying leak I had with the _input or _unhandledInput (can't remember now) functions. But it was nothing major imho just a single inputEvent.

My plan was to revisit the leaks once the rewrite of the memory manager is done: https://github.com/utopia-rise/godot-kotlin-jvm/issues/618

kostaskougios commented 3 months ago

EDIT: thinking a bit more about it, I start godot by redirecting it's output to a log file but I guess my game doesn't start with this redirection, so I need to dig a bit into it and find out how to start the game with the output redirected to my log.

@jsbeckr yes the prob is that it doesn't say even at the console.

I just run godot from the console, run my game, pressed cmd-Q , crash with the popup with jvm leaks, nothing in the console.

But I get this from the OS crash window:

-------------------------------------
Translated Report (Full Report Below)
-------------------------------------

Process:               Godot [5149]
Path:                  /Applications/Godot.app/Contents/MacOS/Godot
Identifier:            org.godotengine.godot
Version:               4.2.2 (4.2.2)
Code Type:             ARM-64 (Native)
Parent Process:        launchd [1]
User ID:               501

Date/Time:             2024-06-23 16:03:48.1927 +0100
OS Version:            macOS 14.5 (23F79)
Report Version:        12
Anonymous UUID:        D8DD00BF-4BFD-31B5-AAF6-82E9E79FFCC0

Sleep/Wake UUID:       12B915A2-CE63-4412-8FAE-B7B3BD9B17C9

Time Awake Since Boot: 8600 seconds
Time Since Wake:       154 seconds

System Integrity Protection: enabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BREAKPOINT (SIGTRAP)
Exception Codes:       0x0000000000000001, 0x0000000102e63590

Termination Reason:    Namespace SIGNAL, Code 5 Trace/BPT trap: 5
Terminating Process:   exc handler [5149]

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   Godot                                  0x102e63590 bridges::MemoryBridge::notify_leak(JNIEnv_*, _jobject*) + 196
1   ???                                    0x175870d94 ???
2   ???                                    0x17586d2d0 ???
3   ???                                    0x175868154 ???
4   libjvm.dylib                           0x12be22244 JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, JavaThread*) + 604
5   libjvm.dylib                           0x12be83ff8 jni_invoke_nonstatic(JNIEnv_*, JavaValue*, _jobject*, JNICallType, _jmethodID*, JNI_ArgumentPusher*, JavaThread*) + 644
6   libjvm.dylib                           0x12be87be8 jni_CallVoidMethodA + 208
7   libjvm.dylib                           0x12bea367c checked_jni_CallVoidMethodA + 268
8   Godot                                  0x102e515b4 jni::JObject::call_void_method(jni::Env&, _jmethodID*, jvalue*) const + 40
9   Godot                                  0x102e471ac GDKotlin::finish() + 860
10  Godot                                  0x105df7a1c ScriptServer::finish_languages() + 196
11  Godot                                  0x10299f144 Main::cleanup(bool) + 484
12  Godot                                  0x10297bc14 main + 312
13  dyld                                   0x19d98a0e0 start + 2360

Thread 1:
0   libsystem_pthread.dylib                0x19dd0dd20 start_wqthread + 0

Thread 2:
0   libsystem_kernel.dylib                 0x19dcd59ec __psynch_cvwait + 8
1   libsystem_pthread.dylib                0x19dd1355c _pthread_cond_wait + 1228
2   libc++.1.dylib                         0x19dc38b14 std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 28
3   Godot                                  0x1059e678c _IP_ResolverPrivate::_thread_function(void*) + 132
4   Godot                                  0x1058f48bc Thread::callback(unsigned long long, Thread::Settings const&, void (*)(void*), void*) + 120
5   Godot                                  0x1058f4bbc 0x1025bc000 + 53709756
6   libsystem_pthread.dylib                0x19dd12f94 _pthread_start + 136
7   libsystem_pthread.dylib                0x19dd0dd34 thread_start + 8
kostaskougios commented 3 months ago

I've redirected the in-game godot log to /tmp/godot.log but it doesn't print any leaks when I cmd-Q:

==> /tmp/godot.log <==
Globals cleaning up
DONE: Globals cleaning up
Globals cleaning up
DONE: Globals cleaning up
Godot-JVM: JVM GC thread was closed
USER ERROR: Godot-JVM: JVM instances are leaking.
   at: notify_leak (modules/kotlin_jvm/src/memory/bridges/memory_bridge.cpp:124)
jsbeckr commented 3 months ago

ah okay... on Linux it looks like this

Godot-JVM: Closing GC thread
Godot-JVM: WARNING: Some JVM godot instances are leaked.
1 Objects:
    InputEventKey -9223371968957380991 C++ instance alive: true
0 Leaked native core types:

(zenity:49197): Adwaita-WARNING **: 23:43:37.820: Using GtkSettings:gtk-application-prefer-dark-theme with libadwaita is unsupported. Please use AdwStyleManager:color-scheme instead.
ERROR: Godot-JVM: JVM instances are leaking.
[...]

Of course it could be a totally different issue.

kostaskougios commented 3 months ago

@jsbeckr hmm, ok I commented out some code of mine that queueFree's some nodes that I dynamically instantiate just to simulate a leak. It didn't log them:

==> /tmp/godot.log <==
Godot-JVM: JVM GC thread was closed
USER ERROR: Godot-JVM: JVM instances are leaking.
   at: notify_leak (modules/kotlin_jvm/src/memory/bridges/memory_bridge.cpp:124)

So maybe this happens only on macos?

kostaskougios commented 3 months ago

Ok got some advice on how to get the leaks into my log and now I can see this when pressing CMD-Q:

==> /tmp/godot.log <==
Godot-JVM: Closing GC thread
Godot-JVM: WARNING: Some JVM godot instances are leaked.
1 Objects:
    InputEventKey -9223371938305404801 C++ instance alive: true
0 Leaked native core types:

I would guess that InputEventKey to be the cmd-q itself, I just run the app and straight away press CMD-Q. My only input handler is not storing any events anywhere (it is for the cam and handles some mouse events), so not sure why that is. I'll put it into my bug entry. It replicates easily by just starting the game and pressing cmd-q.

Note that when I press an "Exit" button that I have in my game that does get_tree().quit(), the game exits without leaks.

kostaskougios commented 3 months ago

A bit more info: today I tried to make "ESC" quit my game. But I get the same memory leak for the InputEventKey:

override fun _input(event: InputEvent) {
        if (event.isActionPressed(QuitSN)) {
            getTree()?.quit()
        }
}

log:

==> /tmp/godot.log <==
Godot-JVM: Closing GC thread
Godot-JVM: WARNING: Some JVM godot instances are leaked.
1 Objects:
    InputEventKey -9223371936963227509 C++ instance alive: true
0 Leaked native core types:

ERROR: Godot-JVM: JVM instances are leaking.
   at: notify_leak (modules/kotlin_jvm/src/memory/bridges/memory_bridge.cpp:124)
kostaskougios commented 3 months ago

one more when changing material. Maybe not multi-threading related:

FATAL ERROR in native method: Invalid global JNI handle passed to DeleteGlobalRef
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.dylib+0x446e90]  checked_jni_DeleteGlobalRef+0x1c8
C  [Godot+0x895430]  jni::JObject::delete_global_ref(jni::Env&)+0x28
C  [Godot+0x88bf70]  JavaInstanceWrapper::swap_to_weak_unsafe()+0x68
C  [Godot+0x898f2c]  KotlinBinding::refcount_decremented_unsafe()+0x40
C  [Godot+0x8991ac]  KotlinBindingManager::_instance_binding_reference_callback(void*, void*, unsigned char)+0x24
C  [Godot+0x38377e4]  RefCounted::unreference()+0xc4
C  [Godot+0x220a2bc]  GeometryInstance3D::set_material_override(Ref<Material> const&)+0xf4
C  [Godot+0x2210bb8]  GeometryInstance3D::~GeometryInstance3D()+0x50
C  [Godot+0x1cbca38]  SceneTree::_flush_delete_queue()+0xe0
C  [Godot+0x1cbcbe4]  SceneTree::process(double)+0x138
C  [Godot+0x3e2608]  Main::iteration()+0x3d8
C  [Godot+0x39179c]  OS_MacOS::run()+0x74
C  [Godot+0x3bfc0c]  main+0x130
C  [dyld+0x60e0]  start+0x938
Sipaha commented 2 months ago

@kostaskougios I discovered that local functions can cause memory leaks.

@RegisterFunction
override fun _ready() {
    val meshInstance = MeshInstance3D()
    fun someInternalFunc() { GD.print("Hello from code with leak!") }
}

This simple code results in the following error:

Godot-JVM: WARNING: Some JVM godot instances are leaked.
1 Objects:
    MeshInstance3D 28655486205 C++ instance alive: true
0 Leaked native core types:

ERROR: Godot-JVM: JVM instances are leaking.
   at: notify_leak (modules/kotlin_jvm/src/memory/bridges/memory_bridge.cpp:124)

If you comment out any line in my example, the error disappears.

kostaskougios commented 2 months ago

@Sipaha yes I noticed that I need to be very careful with the way kotlin (and in my case also scala) work in terms of keeping references of objects. Even now I have an issue where something keeps a ref to an object of mine which in turn keeps refs to godot objects that were queueFree()'d during shutdown, causing it to report memory leaks.

But the bug above is a real bug occurring because for some reason the cmd-q keypress is stored somewhere during shutdown and it causes a leak.

CedNaru commented 1 month ago

661 should solve this.

We no longer handle leaks ourselves and don't crash the engine on purpose anymore.