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

Fix signal handlers #10

Merged
merged 3 commits into from May 19, 2015

Conversation

Projects
None yet
2 participants
@tuomasjjrasanen
Contributor

tuomasjjrasanen commented May 18, 2015

Hi

I noticed couple of minor flaws in the signal handling logic:

  • horst exits with zero when it is signaled to death, should exit with non-zero in my opinion
  • sigint_handler() is unsafe because it calls exit()
  • signal() is used to assign handler functions, which is not portable due to ambiguous semantics

There's three commits in this branch, one commit per flaw.

tuomasjjrasanen added some commits May 15, 2015

Exit with non-zero status if interrupted by a signal
An exceptional/erroneous exit should differ from a normal exit. What would be a
better way to make a difference than using different status values?

0 - user asked for it by pressing 'q'
1 - user did not ask for it but someone killed it e.g. with SIGTERM
Do not call unsafe functions from signal handlers
POSIX defines a list of functions which can safely be interrupted by signals and
re-entered by signal handlers [1]. However, exit() is not included (at least
partially because exit() cannot guarantee functions registered with atexit() are
safe). Calling unsafe functions from signal handlers causes undefined behavior
(in practice deadlocks, heap corruptions etc.).

This commit makes sigint_handler() safe by making it set an atomic flag instead
of calling exit(). The flag is then tested in the mainloop and exit() is called
if the flag was set. Effectively, the burden of signal handling is moved from
the "interrupt context" to the "process context".

Furthermore, a standard pattern of signal masking + pselect() is applied to
avoid race conditions in the signal flag management.

[1]: http://man7.org/linux/man-pages/man7/signal.7.html
Use sigaction() instead of signal()
The semantics of signal() vary across implementations when it is used to set a
handler function for a signal [1]. Let's use well-defined sigaction() instead.

[1]: http://man7.org/linux/man-pages/man2/signal.2.html

br101 added a commit that referenced this pull request May 19, 2015

@br101 br101 merged commit 0260496 into br101:master May 19, 2015

@br101

This comment has been minimized.

Show comment
Hide comment
@br101

br101 May 19, 2015

Owner

Thanks! Very good work!!!

Owner

br101 commented May 19, 2015

Thanks! Very good work!!!

@tuomasjjrasanen tuomasjjrasanen deleted the tuomasjjrasanen:fix_signal_handlers branch Jul 21, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment