double free or corruption (!prev) error on exit (GDNative c++)

[sash-rc]

Godot version:
Godot Engine v3.3.rc7.official

OS/device including version:
Linux Ubuntu 20.04

Issue description:
GDnative C++ application runs fine, but throws error in console: double free or corruption (!prev) on normal application exit.
Not visible when run from editor.

Steps to reproduce:
Any GDnative C++ project, start and exit.

Minimal reproduction project:
Any project, like this one https://github.com/BastiaanOlij/gdnative_cpp_example
Error happens somewhere after Godot::gdnative_terminate(o);

[akien-mga]

Probably caused by #46844, CC @geekrelief @toasteater @Bromeon @godotengine/gdnative

[Bromeon]

Just tried latest versions:

• Godot 3.3.rc.custom_build.34c23002f, 34c2300
• godot-rust/gdnative@4c5184c

I can reproduce it with the dodge_the_creeps example.
Seems like it crashes in nativescript.cpp:953. In the following code, method_data has a value of 0x1 (so off-by-one null pointer):

for (Map<StringName, NativeScriptDesc>::Element *C = classes.front(); C; C = C->next()) {

	// free property stuff first
	for (OrderedHashMap<StringName, NativeScriptDesc::Property>::Element P = C->get().properties.front(); P; P = P.next()) {
		if (P.get().getter.free_func)
			P.get().getter.free_func(P.get().getter.method_data);

		if (P.get().setter.free_func)
			P.get().setter.free_func(P.get().setter.method_data);
	}

Unfortunately I wasn't able to set a breakpoint early enough to see where this comes from. If anyone knows how to launch an external process with debugger attached (not attach to existing one) using CLion, let me know 🙂

[ghost]

method_data has a value of 0x1 (so off-by-one null pointer)

Not really. Stateless methods are ZSTs in Rust so 0x1 is a valid pointer for them. It's probably free_func that is problematic, like last time.

[Bromeon]

Got the Debugger working (normal C++ project, select executable for a CMake run configuration).

I can confirm that the free_func pointer is not valid during NativeScriptLanguage::_unload_stuff(). It seems to be still valid in NativeScriptInstance::set() and NativeScriptInstance::call() during startup. Not sure what exactly happens in between, and the custom hash maps are pretty much impossible to debug... Maybe this information helps someone with a better understanding of that code?

[Edit] debug expression of interest: script_data->properties.list._data->first->value.second.getter.free_func

[geekrelief]

Hey guys, trying to help with debugging the issue, but I can't even get the godot-cpp sample https://github.com/BastiaanOlij/gdnative_cpp_example to compile on Windows. I get errors in Object.hpp.

include\gen\Object.hpp(70): error C2143: syntax error: missing ';' before '<'
include\gen\Object.hpp(70): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
include\gen\Object.hpp(70): error C2238: unexpected token(s) preceding ';'
include\gen\Object.hpp(90): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
include\gen\Object.hpp(90): error C2143: syntax error: missing ',' before '<'

Not sure if it helps, but one thing I did notice was that the merge in #46844 removed the library_classes.erase statement inside of the if(L). I wonder if adding that back would make a difference.

[akien-mga]

Not sure if it helps, but one thing I did notice was that the merge in #46844 removed the library_classes.erase statement inside of the if(L). I wonder if adding that back would make a difference.

What statement exactly? I don't see any removal in https://github.com/godotengine/godot/pull/46844/files

[geekrelief]

Not sure if it helps, but one thing I did notice was that the merge in #46844 removed the library_classes.erase statement inside of the if(L). I wonder if adding that back would make a difference.

What statement exactly? I don't see any removal in https://github.com/godotengine/godot/pull/46844/files

Huh.. So sorry! I guess it must have been my fault. geekrelief@410ae52
In my 3.x branch I have the code there, but in the master branch it's missing. I must have left it out while copying things over.
Edit: I guess this would have been caught if I could have tested this against master. How's gdnative in 4.0 coming along? :)

[Bromeon]

Thanks @geekrelief! The changes in #47623 seem to fix the issue for godot-rust.
Unfortunately I cannot test the PR itself on master, as godot-rust is using 3.x (and will probably do so for a while...).
So I tested the version on branch gdnative-cleanup-3.x.

@sash-rc Not sure if you would be willing to recompile Godot based on my PR and see if that fixes the issue on your system, too?

[sash-rc]

@Bromeon, why not, just another 30 minutes in background terminal :-)
I have built https://github.com/Bromeon/godot/tree/bugfix/gdnative-cleanup-3.x and the given problem is gone.

Thanks.

[sash-rc]

Not very familiar with issues workflow, but probably this one could be closed.

[Calinou]

Not very familiar with issues workflow, but probably this one could be closed.

We should leave this issue open until #47623 is merged 🙂

[Bromeon]

@sash-rc thank you very much for the invested time to check this, it's really appreciated!
I'm glad it works now 🙂