Crash during engine shutdown if get_tree() is ever called from GDNative

[colugomusic]

Godot version:
3.3 stable

OS/device including version:
Windows 10

Issue description:
If get_tree() is ever called from a GDNative plugin then Godot will crash on shutdown when you quit your game.

The crash occurs in NativeScriptLanguage::free_instance_binding_data.

free_instance_binding_data is entered but godot_gdnative_terminate has already been called at this point so the function pointer is invalid.

This doesn't happen in Godot 3.2.

Steps to reproduce:
Call get_tree() from a GDNative class, run your game with a debugger attached and quit.

Minimal reproduction project:
gdnative_get_tree_crash.zip

This is the only relevant part AFAIK:

class Test : public godot::Node {
	GODOT_CLASS(Test, godot::Node);
public:
	static void _register_methods() {
		register_method("_ready", &Test::_ready);
	}
	void _init(){}
	void _ready() {
		get_tree();
		// Godot will now crash in NativeScriptLanguage::free_instance_binding_data() 
	}
};

[colugomusic]

Can confirm this occurs on macOS Big Sur too, where it's worse because the user will be greeted with a crash report window every time they exit.

[akien-mga]

Related to #46844, #47655, and #47623.

CC @godotengine/gdnative @geekrelief @toasteater @Bromeon

[geekrelief]

I can confirm this happens with godot-cpp (Win10 + MSVC, using the cpp gdnative-demos codebase) but doesn't happen with godot-nim or with the C godot-headers. I only have basic familiarity with godot-cpp, but I'll try to help debug this.

[akien-mga]

Note that godot-cpp hasn't been updated yet to match the 3.3-stable release, so that may be part of the problem.

This might get solved once this update is done.

[geekrelief]

Thanks! Hopefully, you spared me the trouble of debugging this. I was in the middle of trying to bisect the godotengine to commits to see which commit was the cause.

[colugomusic]

Note that godot-cpp hasn't been updated yet to match the 3.3-stable release, so that may be part of the problem.

This might get solved once this update is done.

Is there any effort towards figuring out how to synchronize this stuff better? It sounds like there is always going to be a window of several weeks after every Godot release where c++ projects are not going to work. Frustrating for existing developers who do not realise that they need to wait for godot-cpp to catch up, and confusing for new c++ developers who are simply following the instructions in the godot-cpp readme.

[akien-mga]

I can't reproduce the crash on Linux, though if I use a godot-cpp commit before godotengine/godot-cpp#552 and don't upgrade the headers manually, I do get errors:

ERROR: open_dynamic_library: Can't open dynamic library: /home/akien/tmp/godot/bug/gdnative_get_tree_crash/project//bug/bin/x11/libbug.so. Error: /home/akien/tmp/godot/bug/gdnative_get_tree_crash/project//bug/bin/x11/libbug.so: undefined symbol: _ZN5godot25EditorSceneImporterAssimp23___init_method_bindingsEv
   At: drivers/unix/os_unix.cpp:415.
ERROR: get_symbol: No valid library handle, can't get symbol from GDNative object
   At: modules/gdnative/gdnative.cpp:502.
ERROR: init_library: No nativescript_init in "res:///bug/bin/x11/libbug.so" found
   At: modules/gdnative/nativescript/nativescript.cpp:1479.
ERROR: terminate: No valid library handle, can't terminate GDNative object
   At: modules/gdnative/gdnative.cpp:407.

But with that PR merged and the new 3.3 branch it seems to work just fine.

Could you confirm that updating the headers fixes the issue for you?

[colugomusic]

Could you confirm that updating the headers fixes the issue for you?

No, on Windows the same crash is occurring with the new 3.3 branch.

[sisterhooddev]

On Windows 8.1 it's not crashing for me right now.
It (get_tree()->exit()) was crashing for some time at the beginning after I moved project to 3.3 but I think it was VS that didn't fully rebuild project after I updated header files from 3.2 to 3.3???

The problem I got is that godot editor is locking gdnative dll for writing. I have to restart godot editor every time I change dll.
#48086

[colugomusic]

The issue I reported isn't that get_tree() is crashing though. Calling get_tree() at any point simply sets Godot up to crash during program shutdown. get_viewport also causes the crash and probably any other method that goes through the ___godot_icall_Object wrapper. This is still occurring with the 3.3 headers after doing a full clean and rebuild.

Stack trace of the crash:

>	godot.windows.tools.64.exe!NativeScriptLanguage::free_instance_binding_data(void * p_data) Line 1365	C++
 	godot.windows.tools.64.exe!Object::~Object() Line 2049	C++
 	godot.windows.tools.64.exe!MainLoop::~MainLoop() Line 81	C++
 	godot.windows.tools.64.exe!SceneTree::~SceneTree() Line 2176	C++
 	[External Code]	
 	godot.windows.tools.64.exe!memdelete<MainLoop>(MainLoop * p_class) Line 119	C++
 	godot.windows.tools.64.exe!OS_Windows::delete_main_loop() Line 1799	C++
 	godot.windows.tools.64.exe!Main::cleanup(bool p_force) Line 2243	C++
 	godot.windows.tools.64.exe!widechar_main(int argc, wchar_t * * argv) Line 164	C++
 	godot.windows.tools.64.exe!_main() Line 184	C++
 	godot.windows.tools.64.exe!main(int _argc, char * * _argv) Line 196	C++
 	[External Code]	

[sisterhooddev]

This might be stupid question sorry but are you building project by scons file or in VS?

[colugomusic]

The reproduction project I attached in the OP uses cmake to generate a VS project.

[sisterhooddev]

My project was crashing at exit (in export and editor) after some time after updating to 3.3 (editor and headers) and somehow stopped crashing after I was messing with cpp files I think forcing VS to properly rebuild. VS is very buggy.
I am calling get_tree().quit() in gd script, but in many gdnative scripts I have plenty get_viewport()-> calls.

I checked your plugin.cpp file and I think it misses constructor and destructor:
Example:
GDExample();
~GDExample();

EDIT: what is funny project dll compiled with 3.2 headers is also not crashing in Godot 3.3 in Windows

[colugomusic]

The constructor and destructor is irrelevant. The issue is that the plugin's godot_gdnative_terminate has already been called by the time NativeScriptLanguage::free_instance_binding_data is entered so free_instance_binding_data points to an invalid address.

[ghost]

Still relevant on 3.3.1 and on 3.3.2

[akien-mga]

I tried again and I finally managed to reproduce the crash on Linux. It doesn't seem to happen when running the project through the editor, but it does when running standalone.

Here's a fixed MRP so that it builds properly against godot-cpp godot-3.3.3-stable (with renamed godot_headers to godot-headers, and fixed path for Linux library).

gdnative_get_tree_crash.zip

How to use:

cd godot-cpp
cmake -DCMAKE_BUILD_TYPE=Debug .
make -j
cd ..
cmake -DCMAKE_BUILD_TYPE=Debug -DGODOT_CPP_ROOT=godot-cpp .
make -j
cd project
godot

Crash backtrace (from 3.3.4 RC e4df8a6, but that's close enough to 3.3.3 stable):

#0  0x00007fffa01e3497 in ?? ()
#1  0x0000000001bb284f in NativeScriptLanguage::free_instance_binding_data (this=0x7667dc0, p_data=0x7e995a0) at modules/gdnative/nativescript/nativescript.cpp:1363
#2  0x0000000004304168 in Object::~Object (this=0x7adcc60, __in_chrg=<optimized out>) at core/object.cpp:2047
#3  0x00000000044349bc in MainLoop::~MainLoop (this=0x7adcc60, __in_chrg=<optimized out>) at core/os/main_loop.cpp:81
#4  0x000000000348063e in SceneTree::~SceneTree (this=0x7adcc60, __in_chrg=<optimized out>) at scene/main/scene_tree.cpp:2176
#5  0x000000000170fd8d in memdelete<MainLoop> (p_class=0x7adcc60) at ./core/os/memory.h:117
#6  0x0000000001708b12 in OS_X11::delete_main_loop (this=0x7fffffffce20) at platform/x11/os_x11.cpp:2913
#7  0x000000000174522d in Main::cleanup (p_force=false) at main/main.cpp:2241
#8  0x00000000016fa7e9 in main (argc=1, argv=0x7fffffffd728) at platform/x11/godot_x11.cpp:57

[akien-mga]

See also #48086 which seems to be related.

I'd need help from motivated GDNative users to debug and fix this. Alternatively, I can revert the multiple GDNative PRs which led to this regression, but it would reintroduce other issues.

[colugomusic]

See also #48086 which seems to be related.

I'd need help from motivated GDNative users to debug and fix this. Alternatively, I can revert the multiple GDNative PRs which led to this regression, but it would reintroduce other issues.

If you need me to test something let me know. I use GDNative extensively on Windows, Linux and macOS and I can reproduce this crash consistently on all three platforms.

[colugomusic]

It appears this workaround can be added to user code to suppress the crash

extern "C" void GDN_EXPORT godot_gdnative_terminate(godot_gdnative_terminate_options * o)
{
	Godot::nativescript_terminate(godot::_RegisterState::nativescript_handle); // Add this line
	Godot::gdnative_terminate(o);
}

[cromerc]

It appears this workaround can be added to user code to suppress the crash

extern "C" void GDN_EXPORT godot_gdnative_terminate(godot_gdnative_terminate_options * o)
{
	Godot::nativescript_terminate(godot::_RegisterState::nativescript_handle); // Add this line
	Godot::gdnative_terminate(o);
}

I can confirm that this issue is still present even in 3.5 beta 3. And I also confirm that your workaround works for me to suppress the crash.

[polyrain]

Echoing this for 3.4.4, will occur even if get_tree isn't used, was encountering this issue on clean up of ImageTextures and Images generated from a GDNative plugin. Above work around has resolved the issue for me!

[ArdaE]

The issue still exists in Godot 3.5 paired with the 3.5-stable labeled commits of godot-cpp and godot-headers. Any call to get_tree causes the issue later on during exit. @colugomusic's workaround above (call to nativescript_terminate) has fixed the issue for me as well.

Not sure if this would help at all in tracking down this bug, but intentionally leaking any node that has a NativeScript attached to it also prevents the crash on exit.

Furthermore, the stack trace at the time of the crash varies based on other factors. In my case, it's narrowing down the issue to get_tree that led me to this issue report; not the stack trace above.

[and3rson]

Is godotengine/godot-cpp#860 possibly related to this?

#48295 (comment) also fixes it for me.

In my case, segfault happened when both conditions were satisfied:

• A library (which at some point received a Node *node from GDScript as an argument to a function) was unloaded
• The passed node appeared earlier in the tree than the node with GDN-ative attached.

Strangely, doing node->free() within the GDNative library prevented the crash from happening, but was in fact just a nasty lucky side-effect.
Additionally, moving the node (that was passed into GDNative lib) lower in the tree (under GDNative lib node) also prevented the crash from happening.
(See godotengine/godot-cpp#860 for a more detailed STR)