Memory Leak on open_db(), close_db()

[Cryptoclysm]

Environment:

• OS: Windows 10, Debian 11
• Godot version: 3.5
• godot-sqlite version: 3.3

Issue description: Static memory usage goes up permanently after using

db.open_db()
db.close_db()

I estimate that each pair of calls generates around 90 Bytes of leaked memory.

This is a major problem for game servers which may be open for long periods of time.

A workaround could be to keep the db open and only close on application close, but this is not possible for use cases such as mine, where Litestream is used for backups, and requires occasional db locks.

Steps to reproduce: Call:

db.open_db()
db.close_db()

Repeatedly, then view the static memory usage in the Godot debugger. I've provided an example minimal project which exhibits this well.

Minimal reproduction project: MemoryLeakDemo.zip

This project opens and closes a database file 100 times per process frame.

[2shady4u]

Hi @Cryptoclysm,

This seems to be the culprit:

const char *char_path = path.alloc_c_string();

This method allocates memory on the heap so it doesn't get cleared when the stack pops...

[2shady4u]

Changing the following code:

const char *char_path = path.alloc_c_string();

to:

const CharString dummy_path = path.utf8();
const char *char_path = dummy_path.get_data();

Fixes the memory leakage 🥳 Kinda sucks that alloc_c_string() allocates memory on the heap...

EDIT: This also works:

const char *char_path = path.alloc_c_string();
godot::api->godot_free((void *)char_path);

[Cryptoclysm]

@2shady4u Great find! I've applied the patch and rebuilt locally. The line is looking as flat as it gets now!

I've not had a look into any other functions, but this fix will save a lot of leaked memory since it's within db_open().

[2shady4u]

Hi @Cryptoclysm,

I'll try to release a new build at some point that fixes the issue... (hopefully soonish) Also gotta figure out which approach I will take 🤔

[2shady4u]

Hi @Cryptoclysm,

I'm having some major issues with GDNative's implementation of the Godot::print() and String()-methods which seem to overwrite my char* variables.

I've opened an issue here: https://github.com/godotengine/godot-cpp/issues/829

This will unfortunately delay the fix of this memory leak somewhat 😞

[Cryptoclysm]

Hi @2shady4u,

Thanks for the update, looks like we've opened a can of worms. If it's an issue on Godot's side, then I'd assume the fix would not be compatible with older versions of Godot. Would the workaround mentioned in https://github.com/godotengine/godot/issues/40957 be appropriate in this case?

[2shady4u]

Hi @2shady4u,

Thanks for the update, looks like we've opened a can of worms. If it's an issue on Godot's side, then I'd assume the fix would not be compatible with older versions of Godot. Would the workaround mentioned in godotengine/godot#40957 be appropriate in this case?

Yeah, I just have to add godot::api->godot_free((void *)my_char_ptr); everywhere... 🤓 (Or I can slap some goto on there, but let's not go that way!)

Also: this memory leak nor this memory corruption issue shouldn't exist in the gdextension-branch.

[2shady4u]

I'll push some fixes on the new fix-memory-leakage-branch (as found here).

EDIT: I'm not getting any memory leakage anymore when running the demo-project in that specific branch. Unfortunately, it is also looking quite hideous imo.

[Cryptoclysm]

@2shady4u Thanks, I've compiled the branch on Windows and it's looking good for me too. I can't seem to get the .so file to compile for linux for some reason - it's only generating a dll. I'll have another go next week.

EDIT: All working great now under Linux, memory usage on my server has gone from 200MB after 1 hr of running to a stable 50MB and not growing hurrah! dll issue was solved by compiling on Linux rather than trying to compile for Linux on Windows.

[2shady4u]

Hi @Cryptoclysm,

I've merged the fix-memory-leakage-branch into the master-branch. I'll close this issue when I'll do a new release.

[2shady4u]

The fix of this issue has now been published in release v3.4! 🥳