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

Use sigsetjmp to continue cleanup after SIGTERM/SIGINT #148

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matthewbauer
Copy link
Contributor

This uses the non-local goto sigsetjmp function to continue cleaning
up after a SIGINT or SIGTERM has been caught. Normally, SIGINT or
SIGTERM signal the end of the program and cannot be ignored. We jump
to the cleanup in the signal handler.

I think this handles half of the problem in #146

/cc @jbeich @Hjdskes

This uses the non-local goto sigsetjmp function to continue cleaning
up after a SIGINT or SIGTERM has been caught. Normally, SIGINT or
SIGTERM signal the end of the program and cannot be ignored. We jump
to the cleanup in the signal handler.
@jbeich
Copy link
Contributor

jbeich commented May 19, 2020

I confirm, Cage now respects Ctrl+C (SIGINT) and default kill signal (SIGTERM).

@Hjdskes
Copy link
Collaborator

Hjdskes commented May 24, 2020

Can you elaborate a bit on how this works and why this is needed, @matthewbauer?

@matthewbauer
Copy link
Contributor Author

matthewbauer commented May 24, 2020

The problem is that Wayland sets SIG_BLOCK (https://github.com/wayland-project/wayland/blob/df969706f4cd479a525a69a13f86589d6b313b5b/src/event-loop.c#L731). You might say this is a bug in Wayland since you would expect the event loop to exit properly, but I think there is a reason for this SIG_BLOCK in Wayland. Moreover, you might be able to override it so that SIGINT / SIGTERM are SIG_UNBLOCK, but again I'm not sure if this could negatively effect the Wayland event loop (Note: I originally assumed that SIG_BLOCK was the default behavior, but it looks like this is explicitly set in Wayland, and SIG_UNBLOCK is the default).

More info on the behavior of SIG_BLOCK / blocking signals is available here:

http://man7.org/linux/man-pages/man7/signal.7.html

setjmp saves all its scope and returns 0. When you call longjmp, the program returns to the setjmp call, this time returning non-zero, and unpacks all of the scope. This works exactly like a goto, but beyond the scope boundaries. I am unsure of the actual difference between setjmp/longjmp and sigsetjmp/siglongjmp, besides that sigsetjmp/siglongjmp is supposed to be used in signal handlers.

The alternative to the above would be making all of the state global and moving everything that is done after wl_display_run into a cleanup function. The signal handler would just call that instead of using longjmp. Basically same effect, but in this case you have to make your scope global.

@Hjdskes
Copy link
Collaborator

Hjdskes commented May 31, 2020

Thanks for the explanation @matthewbauer! The code itself makes sense and I have no concerns around that specifically, but I wonder if this is just a workaround rather than an actual solution. To my understanding, other wlroots-based compositors do not need this workaround. I cannot promise when, but I will investigate this some more before I merge your PR. Unless you beat me to it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants