Cataclysm-DDA
Cataclysm-DDA copied to clipboard
main: properly handle SIGINT
Summary
None
Purpose of change
- We're handling SIGINT (Ctr+C) by killing ourselves
- Killing ourselves doesn't immediately terminate execution
- Sometimes the OS doesn't terminate us in time and execution proceeds as if SIGINT was cancelled and the game tries to refresh display, leading to a segfault since we've already manually destroyed some game objects.
- Additionally, the sig handler does not comply with MSC54-CPP, so we've been relying on UB this entire time
-
Fixes: #8417
-
Fixes: #77011
-
Closes: #76173
-
Closes: #77050 these both do similar things in that they try to move execution to a less crashy path before the OS takes over, but don't address the core issue.
Describe the solution
Split SIGINT handling to a separate function
Move all non-SIG30-C-compliant code out of the sig handler
Add a new quit reason (QUIT_EXIT) and pass it up to the main loop as an exception, then exit gracefully.
Describe alternatives you've considered
Give up on pedantry and just revert #67893 since the old code worked fine and for all edge cases despite being wrong.
Instead of passing an exception, I could pepper QUIT_EXIT checks all over the place.
Testing
Send SIGINT or other signals, or try to close the window while playing the game in various states: opening menu, loading files, finalizing data, verifying data, loading save file, waiting for input, doing an activity, and in various UIs. The game should prompt and quit correctly with no crashes.
Additional context
It can take a long time for the prompt to appear during some init stages. More QUIT_EXIT checks can be peppered around the code if needed.
UIs that don't use a QUIT keybinding won't react to SIGINT.
There might be other edge cases. Doing it correctly without an unified input loop is hard work.
This fixes SIGINT, but the linked issue only talks about SIGTERM. I need to double check that
Could close the 10 years old #8417 too ?
Does this fix the root cause of the heap overflow, causing corrupted crash path locations?
Could close the 10 years old #8417 too ?
It does now, thanks.
Does this fix the root cause of the heap overflow, causing corrupted crash path locations?
This PR turned out to fix an unrelated crash.
to be honest I dont think my PR should be invalidated until theres a proper fix for the mystery heap overflow. I already did everything I can to debug this.
to be honest I dont think my PR should be invalidated until theres a proper fix for the mystery heap overflow. I already did everything I can to debug this.
There is no mystery. The crash log shows the problem fairly clearly. There are just multiple, unrelated instances of the same issue. In any case, your PR just hides the issue under the rug as I've tried to explain on discord as well.
So the other PR does fix the underlying issue leading to the heap overflow?
It seems like the quit dialog gets stuck if you send another sigint while it's open.
Otherwise from some quick testing this seems to work as expected :+1:
NB: only tested this on tiles
It seems like the quit dialog gets stuck if you send another sigint while it's open.
Thanks for catching that. It should be fixed now.
I can still reproduce it, eg. on tiles, right on the main menu before loading the game, after two sigints nothing can be selected in the dialog.
Also I noticed on the main menu, when selecting No, the game still closes.
I can still reproduce it, eg. on tiles, right on the main menu before loading the game, after two sigints nothing can be selected in the dialog.
Also I noticed on the main menu, when selecting No, the game still closes.
Fixed. I'll leave this in draft for a bit, in case you find other things. Thanks a lot for testing!
Tested both tiles and curses, seems to work fine now :+1:
Thanks for merging.