Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling exception crashes editor/program #1326

Closed
id01 opened this issue Dec 3, 2023 · 15 comments
Closed

Handling exception crashes editor/program #1326

id01 opened this issue Dec 3, 2023 · 15 comments
Labels
archived bug This has been identified as a bug crash topic:gdextension This relates to the new Godot 4 extension implementation

Comments

@id01
Copy link

id01 commented Dec 3, 2023

Godot version

4.2.stable

godot-cpp version

4.2.stable

System information

Arch Linux

Issue description

A handled exception in C++ crashes the Godot editor and the program.

Example code causing crash:

class Errorer : public Node {
	GDCLASS(Errorer, Node);
protected:
	static void _bind_methods() {
	}

public:
	Errorer() {
		try {
			UtilityFunctions::print("Throwing exception");
			std::unordered_map<int, int> a;
			a.at(1);
		} catch (std::out_of_range e) {
			UtilityFunctions::print("Caught!");
		}
	}
	~Errorer() = default;
};

When this code is put in a GDExtension, the editor crashes with SIGABRT.

Relevant traceback portion:

#1  0x00007ffff536f668 in raise () from /usr/lib/libc.so.6
#2  0x00007ffff53574b8 in abort () from /usr/lib/libc.so.6
#3  0x00007ffff79df056 in _Unwind_SetGR (context=<optimized out>, index=<optimized out>, val=<optimized out>) at /usr/src/debug/gcc/gcc/libgcc/unwind-dw2.c:280
#4  0x00007ffff4caf9f3 in __cxxabiv1::__gxx_personality_v0 (version=<optimized out>, actions=6, exception_class=5138137972254386944, ue_header=<optimized out>, 
    context=0x7fffffffc9e0) at /usr/src/debug/gcc/gcc/libstdc++-v3/libsupc++/eh_personality.cc:724
#5  0x00005555598b4177 in ?? ()
#6  0x00005555598b49f0 in ?? ()
#7  0x00005555598034be in __cxa_throw ()
#8  0x00007ffff4ca021d in std::__throw_out_of_range (__s=0x7ffff2bc382a "unordered_map::at") at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/functexcept.cc:86
#9  0x00007ffff2b44427 in Errorer::Errorer() () from /home/user/Desktop/Programming/games/error/simulation/library.linux.template_debug.x86_64.so
#10 0x00007ffff2b446a0 in Errorer::create(void*) () from /home/user/Desktop/Programming/games/error/simulation/library.linux.template_debug.x86_64.so

I don't think it should cause a crash when disable_exceptions=false is specified in scons

Steps to reproduce

Compile the minimal reproduction project with

scons platform=linux disable_exceptions=false

Open the minimal reproduction project in the editor

Minimal reproduction project

archive.zip

@AThousandShips
Copy link
Member

Are you building the editor with errors enabled as well?

@AThousandShips AThousandShips added bug This has been identified as a bug crash needs testing topic:gdextension This relates to the new Godot 4 extension implementation labels Dec 3, 2023
@id01
Copy link
Author

id01 commented Dec 3, 2023

Are you building the editor with errors enabled as well?

I used the default binary editor. Looks like in the 4.2 update, they updated disable_exceptions from false -> true by default, so none of the provided binary distributions work. However, it works if I compile the engine myself with disable_exceptions=false.

@AThousandShips
Copy link
Member

Then the question is if we need some way to handle errors with the engine built without, or if it's not possible to support errors without the binary also supporting it

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 6, 2023

I'm not able to reproduce this using the same code in the issue description. (Although, I didn't use the MRP from the ZIP, I copy-pasted the code into my own local project.)

For me, creating an instance of the Errorer class in GDScript doesn't cause a crash, and the expected output is printed to the console:

Throwing exception
Caught!

I'm using the latest godot-cpp master branch together with the official Godot 4.2-stable on Linux (Ubuntu 22.04) with gcc 11.4.0.

I'm not sure what could be the difference could be between your setup and mine?

@kb173
Copy link

kb173 commented Dec 20, 2023

Compiling Godot with disable_exceptions=false also fixed issues I've been getting since Godot 4.2. Thanks!

I think the default of disable_exceptions=true is a bit problematic when it comes to GDExtensions though. One of their huge benefits is the ability to integrate other C++ libraries - we rely on that heavily in our plugin for geospatial data Geodot, someone else reported problems with OpenCV, and I'm sure there are other GDExtensions with similar problems. I know that compiling Godot manually is possible, but it's not feasible for everyone, and it'd be a shame if our GDExtension is only available to people who go through the effort of manual Godot compilation from now on.

Is there any way to avoid the breakage caused by the default disable_exceptions=true? If not, are there strong reasons in favor of keeping that change? Alternatively, is it possible to provide official binaries with disable_exceptions=false?

@AThousandShips
Copy link
Member

Please read the conversation above, this is the question being investigated 🙂

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 20, 2023

@kb173 Can you share a project that reproduces this issue? I tried reproducing using the code from the OP, and I'm able to compile a GDExtension that uses exceptions which is loaded by the official Godot binaries (4.2-stable) which have exceptions disabled, and everything seems to work correctly.

@Calinou
Copy link
Member

Calinou commented Dec 20, 2023

If not, are there strong reasons in favor of keeping that change?

Disabling exception handling speeds up compilation and reduces binary size considerably. See comparisons on Godot itself: godotengine/godot#80612

Ideally, C++ libraries themselves shouldn't use exceptions for better portability (consider this if you're writing your own libraries). You should only pay for the performance cost of features you actually need 🙂

@kb173
Copy link

kb173 commented Dec 20, 2023

If not, are there strong reasons in favor of keeping that change?

Disabling exception handling speeds up compilation and reduces binary size considerably. See comparisons on Godot itself: godotengine/godot#80612

Ideally, C++ libraries themselves shouldn't use exceptions for better portability (consider this if you're writing your own libraries). You should only pay for the performance cost of features you actually need 🙂

I see. I'm not using exceptions myself though, as noted in #1341 and godotengine/godot#85959, basic C++ functionality like writing to std::cout breaks with the default flag. Also, many libraries depend on extensions. So if there's any way to disable exceptions on the Godot side, but enable them in the GDExtension, that would be great.

@kb173 Can you share a project that reproduces this issue? I tried reproducing using the code from the OP, and I'm able to compile a GDExtension that uses exceptions which is loaded by the official Godot binaries (4.2-stable) which have exceptions disabled, and everything seems to work correctly.

I didn't do any testing with actually throwing exceptions since I got here from #1341, but running the test project from this repository breaks for me unless I use a manually compiled Godot binary with the following change:

diff --git a/test/src/example.cpp b/test/src/example.cpp
index 5372d70..5600869 100644
--- a/test/src/example.cpp
+++ b/test/src/example.cpp
@@ -13,6 +13,8 @@
 #include <godot_cpp/classes/multiplayer_peer.hpp>
 #include <godot_cpp/variant/utility_functions.hpp>
 
+#include <iostream>
+
 using namespace godot;
 
@@ -288,6 +291,7 @@ Example::~Example() {
 
 // Methods.
 void Example::simple_func() {
+       std::cout << "simple func" << std::endl;
        emit_custom_signal("simple_func", 3);
 }

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 20, 2023

I didn't do any testing with actually throwing exceptions since I got here from #1341, but running the test project from this repository breaks for me unless I use a manually compiled Godot binary with the following change:

@kb173 I just tried using your patch and running the test project, and it worked fine for me :-/

I'm on Ubuntu 22.04, using GCC 11.4 and running with the stock Godot 4.2.1-stable binary (so not compiled with any special arguments). I compiled the test GDExtension with just plain scons, I didn't even use scons disable_exceptions=no for the GDExtension itself.

And when I run the test project in test/project, it doesn't crash, and, in fact, correctly prints "simple func" to the console.

Here's my full output:

Godot Engine v4.2.1.stable.official.b09f793f5 - https://godotengine.org
WARNING: XOpenIM failed
     at: DisplayServerX11 (platform/linuxbsd/x11/display_server_x11.cpp:5951)
WARNING: XCreateIC couldn't create wd.xic
     at: _create_window (platform/linuxbsd/x11/display_server_x11.cpp:5575)
Vulkan API 1.3.242 - Forward+ - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce RTX 4070 Ti
 
simple func

 ==== TESTS FINISHED ==== 

   PASSES: 100
   FAILURES: 0

 ******** PASSED ******** 

So, there is some difference that you, @Vantskruv and @id01 are encountering, and we really need to figure out what it is, so we can give proper direction to users, or fix the bug if one exists. Neither your example, nor the original one here that actually uses exceptions, should be crashing.

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 20, 2023

Looking over the 3 issues, it seems like all were on Arch Linux or descendant (one was Manjaro which is based on Arch). I wonder if that is somehow related? When I get a chance, I'm going to setup a VM with Arch and see if I can reproduce it there.

@kb173
Copy link

kb173 commented Dec 21, 2023

Looking over the 3 issues, it seems like all were on Arch Linux or descendant (one was Manjaro which is based on Arch). I wonder if that is somehow related? When I get a chance, I'm going to setup a VM with Arch and see if I can reproduce it there.

Oooh, your hunch about it being Arch Linux related seems to be right. I just tested my reproduction patch with v4.2.1.stable.official.b09f793f5 (downloaded from the website) and it works fine, it's printing "simple func" and not crashing - even without any disable_exceptions flag. So the issue seems to be with the Godot binary in the Arch repository, v4.2.1.stable.arch_linux (which is crashing on the exact same project with the same GDExtension binaries).

I guess the whole disable_exceptions thing may have been a placebo then, and any manually compiled Godot binary would've worked? We'll need to do some more testing, maybe @Vantskruv and @id01 can also test and confirm if the official Godot binary from godotengine.org is working fine? If so, this issue can probably be moved to the Arch package repository.

@id01
Copy link
Author

id01 commented Dec 29, 2023

@kb173 I just tested the Binary distribution from Godot's site directly. It works. Then, I tried the Arch version. It doesn't. This is for 4.2.1 this time.
Looks like this is an issue with the Arch repos.

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 29, 2023

@kb173 @id01 Thanks for testing!

If someone opens an issue with Arch, I don't know where they they keep their issues, but if you could put a link to the issue in a comment here so we can follow its progress, that would be really great :-)

EDIT: This looks like the place to report an issue, however, so far I haven't managed to create an account so that I can post one :-/

@kb173
Copy link

kb173 commented Jan 2, 2024

@kb173 @id01 Thanks for testing!

If someone opens an issue with Arch, I don't know where they they keep their issues, but if you could put a link to the issue in a comment here so we can follow its progress, that would be really great :-)

EDIT: This looks like the place to report an issue, however, so far I haven't managed to create an account so that I can post one :-/

I opened an issue there: https://gitlab.archlinux.org/archlinux/packaging/packages/godot/-/issues/2

Let me know if I missed anything in the description!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived bug This has been identified as a bug crash topic:gdextension This relates to the new Godot 4 extension implementation
Projects
None yet
Development

No branches or pull requests

5 participants