Bug Report:Memory leak with passing lua functions to GDScript

[RadiantUwU]

Describe the bug This bug happens by creating constant references and not being able to know when the reference is removed or not.

To Reproduce

extends Node

func do_something_with_callable(callable: Callable):
    pass

# Called when the node enters the scene tree for the first time.
func _ready():
    var api = LuaAPI.new()
    api.push_variant("do_something_with_callable",do_something_with_callable)
    api.do_string("""
    while true do
        do_something_with_callable(function() end)
    end
    """)

Expected behavior The memory usage should remain stable, not constantly rising.

Environment (please complete the following information):

• OS: Linux 6.4.12-arch1-1
• Godot version: v4.1.1.stable.official [bd6af8e0e]
• Module version: v2.1-beta4

Additional context Add any other context about the problem here. Very quickly rising memory usages from 200 MB to 500MB within 5 seconds, potentially more if left longer.

[Trey2k]

So this is a known memory leak, the issue is without the CallableCustom type we have no way to have a deconstructor tied to the callable, meaning a copy of the lua function remains on the state.

This should hopefully be resolved once https://github.com/godotengine/godot/pull/79005 is merged, and we can use the CallableCustom type.

[RadiantUwU]

[RadiantUwU]

So this is a known memory leak, the issue is without the CallableCustom type we have no way to have a deconstructor tied to the callable, meaning a copy of the lua function remains on the state.

This should hopefully me resolved once godotengine/godot#79005 is merged, and we can use the CallableCustom type.

Make a new refcounted and pass it as the instance for creating the callable, give it the reference and make it so when the refcounted disappears it clears it from the registry.

[RadiantUwU]

Should i try doing a pull request...?

[Trey2k]

Make a new refcounted and pass it as the instance for creating the callable, give it the reference and make it so when the refcounted disappears it clears it from the registry.

This is something I had attempted, feel free to try. But check out this issue as well https://github.com/godotengine/godot/issues/74990

[dsnopek]

If you could test the CustomCallable PR (https://github.com/godotengine/godot/pull/79005) with this language binding and report whether or not it was sufficient for your needs, that could help get it merged. :-) The main thing holding back that PR at the moment, is that we're waiting for other language bindings to try it out and give feedback.

[Trey2k]

If you could test the CustomCallable PR (godotengine/godot#79005) with this language binding and report whether or not it was sufficient for your needs, that could help get it merged. :-) The main thing holding back that PR at the moment, is that we're waiting for other language bindings to try it out and give feedback.

Sure thing! Ill give it a test later this evening and report back.

[Trey2k]

@dsnopek I am having some trouble testing the PR, I maintain a fork of godot-cpp with https://github.com/godotengine/godot-cpp/pull/1017 applied, without this PR LuaAPI will not compile at all. There is no workaround I can apply that I know of. This is another big one we have been waiting on for awhile. The issue I am having is there seems to be a lot of merge conflicts and blindly accepting incoming changes does not seem work.

I don't have that much time to dig through the actual issues at the moment sadly, i might give it some more time this weekend.

[RadiantUwU]

@dsnopek I am having some trouble testing the PR, I maintain a fork of godot-cpp with godotengine/godot-cpp#1017 applied, without this PR LuaAPI will not compile at all. There is no workaround I can apply that I know of. This is another big one we have been waiting on for awhile. The issue I am having is there seems to be a lot of merge conflicts and blindly accepting incoming changes does not seem work.

I don't have that much time to dig through the actual issues at the moment sadly, i might give it some more time this weekend.

Wish i could fix the merge conflicts since i sure as hell dont have anything to do.

[Trey2k]

Wish i could fix the merge conflicts since i sure as hell dont have anything to do.

Feel free, my failed attempt is here https://github.com/Trey2k/godot-cpp/tree/CallableCustom

[dsnopek]

I am having some trouble testing the PR, I maintain a fork of godot-cpp with https://github.com/godotengine/godot-cpp/pull/1017 applied, without this PR LuaAPI will not compile at all.

PR https://github.com/godotengine/godot-cpp/pull/1017 needs to get rebased and updated for the review I gave last month before it can be merged. The creator hasn't responded, so I've been thinking about taking that one over in a new PR, but haven't had a chance to do it yet. It is definitely an issue I'd like to see fixed!

[Trey2k]

@dsnopek Thank you for all the work you are doing. I have a small update for you, I have gotten https://github.com/godotengine/godot-cpp/pull/1155 to merge with https://github.com/godotengine/godot-cpp/pull/1238 which now builds godot-cpp without issue. I have also compiled Godot with https://github.com/godotengine/godot/pull/79005 and extracted the GDExtension interface.

I am not sure if I am overlooking something or what, but no class for CallableCustom seems to generate to be able to extend from. Does this indicate something went wrong during the merge, or are we supposed to create CallableCustoms in a different way then within Godot itself? You can find the fork of godot-cpp I am working on here if it is any help https://github.com/Trey2k/godot-cpp/tree/CallableCustom

[dsnopek]

Ah, so the Godot PR adds the API to gdextension_interface.h to create custom callables, but the godot-cpp PR only uses that API to implement callable_mp() and callable_mp_static(), it doesn't provide a base class for making your own custom callables. But that's a good idea! When I have a chance I think I'll try to extend the godot-cpp PR to include something like that.

However, in the meantime, you could use the APIs added to gdextension_interface.h: basically, you make a GDExtensionCallableCustomInfo struct and pass it to the godot::internal::gdextension_interface_callable_custom_create() function.

[Trey2k]

However, in the meantime, you could use the APIs added to gdextension_interface.h: basically, you make a GDExtensionCallableCustomInfo struct and pass it to the godot::internal::gdextension_interface_callable_custom_create() function.

Makes sense, I will attempt this evening and report back. Thank you!

[Trey2k]

I think my merge is bad, the test fail to build with

 -DNDEBUG -I/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/gdextension -I/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include -I/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/gen/include -I/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/src /home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/gen/src/classes/texture3drd.cpp
In file included from src/register_types.h:9,
                 from src/register_types.cpp:6:
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp: In instantiation of 'static void godot::ClassDB::_register_class(bool) [with T = ExampleRef; bool is_abstract = false]':
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:212:36:   required from 'static void godot::ClassDB::register_class(bool) [with T = ExampleRef]'
src/register_types.cpp:24:37:   required from here
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:181:39: error: invalid conversion from 'void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool)' {aka 'void (*)(void*, int, unsigned char)'} to 'GDExtensionClassNotification2' {aka 'void (*)(void*, int, bool)'} [-fpermissive]
  181 |         GDExtensionClassCreationInfo2 class_info = {
      |                                       ^~~~~~~~~~
      |                                       |
      |                                       void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool) {aka void (*)(void*, int, unsigned char)}
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp: In instantiation of 'static void godot::ClassDB::_register_class(bool) [with T = ExampleMin; bool is_abstract = false]':
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:212:36:   required from 'static void godot::ClassDB::register_class(bool) [with T = ExampleMin]'
src/register_types.cpp:25:37:   required from here
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:181:39: error: invalid conversion from 'void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool)' {aka 'void (*)(void*, int, unsigned char)'} to 'GDExtensionClassNotification2' {aka 'void (*)(void*, int, bool)'} [-fpermissive]
  181 |         GDExtensionClassCreationInfo2 class_info = {
      |                                       ^~~~~~~~~~
      |                                       |
      |                                       void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool) {aka void (*)(void*, int, unsigned char)}
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp: In instantiation of 'static void godot::ClassDB::_register_class(bool) [with T = Example; bool is_abstract = false]':
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:212:36:   required from 'static void godot::ClassDB::register_class(bool) [with T = Example]'
src/register_types.cpp:26:34:   required from here
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:181:39: error: invalid conversion from 'void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool)' {aka 'void (*)(void*, int, unsigned char)'} to 'GDExtensionClassNotification2' {aka 'void (*)(void*, int, bool)'} [-fpermissive]
  181 |         GDExtensionClassCreationInfo2 class_info = {
      |                                       ^~~~~~~~~~
      |                                       |
      |                                       void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool) {aka void (*)(void*, int, unsigned char)}
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp: In instantiation of 'static void godot::ClassDB::_register_class(bool) [with T = ExampleVirtual; bool is_abstract = false]':
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:212:36:   required from 'static void godot::ClassDB::register_class(bool) [with T = ExampleVirtual]'
src/register_types.cpp:27:41:   required from here
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:181:39: error: invalid conversion from 'void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool)' {aka 'void (*)(void*, int, unsigned char)'} to 'GDExtensionClassNotification2' {aka 'void (*)(void*, int, bool)'} [-fpermissive]
  181 |         GDExtensionClassCreationInfo2 class_info = {
      |                                       ^~~~~~~~~~
      |                                       |
      |                                       void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool) {aka void (*)(void*, int, unsigned char)}
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp: In instantiation of 'static void godot::ClassDB::_register_class(bool) [with T = ExampleAbstract; bool is_abstract = true]':
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:217:35:   required from 'static void godot::ClassDB::register_abstract_class() [with T = ExampleAbstract]'
src/register_types.cpp:28:51:   required from here
/home/trey2k/Development/WeaselGames/modules/luaAPI/external/godot-cpp/include/godot_cpp/core/class_db.hpp:181:39: error: invalid conversion from 'void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool)' {aka 'void (*)(void*, int, unsigned char)'} to 'GDExtensionClassNotification2' {aka 'void (*)(void*, int, bool)'} [-fpermissive]
  181 |         GDExtensionClassCreationInfo2 class_info = {
      |                                       ^~~~~~~~~~
      |                                       |
      |                                       void (*)(GDExtensionClassInstancePtr, int32_t, GDExtensionBool) {aka void (*)(void*, int, unsigned char)}

I don't know enough about the inner working of GDExtension yet to really debug this myself, but if I get more time I will try to dig into it.

[RadiantUwU]

@Trey2k Seems like 2.1-stable will only be released for 4.2+

[RadiantUwU]

https://github.com/godotengine/godot/pull/79005 And this got merged as well, amazing

[Trey2k]

Seems like 2.1-stable will only be released for 4.2+

This will probably be the case unless the change is back ported yes.

[Trey2k]

Fixed for module based builds in #169

[Trey2k]

I wouldn't call this fully resolved until CallableCustoms actually work for GDExtension.

[dsnopek]

Ah, so the Godot PR adds the API to gdextension_interface.h to create custom callables, but the godot-cpp PR only uses that API to implement callable_mp() and callable_mp_static(), it doesn't provide a base class for making your own custom callables. But that's a good idea! When I have a chance I think I'll try to extend the godot-cpp PR to include something like that.

Sorry it took me so long to circle back to this, but there is now a PR to godot-cpp that makes it easier to create your own custom callables (by extending the CallableCustom class):

https://github.com/godotengine/godot-cpp/pull/1280

Please give it a try when you have a chance, and let me know if it works for you!

[michieal]

I'm very hopeful for this, as I have encountered this bug myself, and wondered what had caused it. It's especially noticeable when running your game in debug mode from your IDE instead of from the editor.