Repository navigation
Conversation
exit() is not async-signal-safe (POSIX) and runs the atexit(cleanup) handler, which tears down the X11 connection, unix socket, and devices. After a real segfault, process state may be corrupted, and running that same teardown code could itself crash or hang - counterproductive for a handler whose whole point is to "exit gracefully". SIGINT/SIGTERM keep using exit() as before, so normal shutdown still runs cleanup() and releases everything properly. Only SIGSEGV now calls _exit() directly. This also resolves the -Wimplicit-fallthrough warning under -Wextra, since SIGSEGV no longer falls through into the other cases.
Contributor
|
Correct me if I'm wrong, but doesn't that make attempting to handle SIGSEGV completely pointless? It was an attempt to maybe get the chance to clean some things up before exiting, if the situation allows it. But with your change we'll just exit immediately without any cleanup, which is what the default SIGSEGV handler would do anyway. |
Author
|
Sorry for the noise — you're right, I hadn't considered that trading a chance at cleanup for a guaranteed no-op isn't actually an improvement. Closing this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
`exit()` is not async-signal-safe (POSIX) and runs the `atexit(cleanup)` handler, which tears down the X11 connection, unix socket, and devices. After a real segfault, process state may be corrupted, and running that same teardown code from within the signal handler could itself crash or hang - somewhat counterproductive for a handler whose whole point is to "exit gracefully".
This only changes the `SIGSEGV` case to call `_exit()` directly. `SIGINT`/`SIGTERM` keep using `exit()` as before, so normal shutdown still runs `cleanup()` and releases everything properly - only the segfault path skips it.
As a side effect, this also resolves the `-Wimplicit-fallthrough` warning under `-Wextra`, since `SIGSEGV` no longer falls through into the other cases (found while looking at the codebase with stricter warnings enabled, unrelated to the fix itself).
Small, self-contained change - happy to adjust if you'd prefer a different approach.