GNOME Bugzilla – Bug 704699
[PATCH] gmain: Reset signal handlers to default when source is
Last modified: 2018-05-24 15:32:01 UTC
If someone creates a unix signal source for e.g. SIGINT, and then removes it, reset the handler to SIG_DFL. Not doing this was the source of race conditions in the glib/tests/unix test, but this will also just make us a "good citizen" by cleaning up. For example, if a project temporarily creates a handler for SIGTERM, and then later removes it, they almost certainly want SIGTERM to revert to the default of terminating the process, rather than doing nothing. --- glib/gmain.c | 51 +++++++++++++++++++++++++++++++-------------------- glib/tests/unix.c | 11 ++++------- 2 files changed, 35 insertions(+), 27 deletions(-)
Created attachment 249829 [details] [review] reset signal handlers
Comment on attachment 249829 [details] [review] reset signal handlers hm... really we ought to call unref_unix_signal_handler_unlocked() when the source is destroyed, not when it's finalized (in case some GC'ed runtime is still holding a ref on the source or whatever). But... GSource doesn't let us do that, so, oh well. Better than nothing.
<desrt> this patch, on the other hand is insanely racy: https://bugzilla.gnome.org/show_bug.cgi?id=704699 * desrt is slightly upset by the comments like "oh well, better than nothing" and "this is still racy" tossed around during the review processes of these patches and the continuous unbreaking that is necessitated by them <desrt> walters: in the reset-sighandlers one, there is a race caused by destroying a signal handler that already has a signal queued for dispatch (or one that is already on its way to the worker thread but not yet dispatched to the source) <desrt> so two races, i guess <desrt> taking SIGTERM as an example -- if the user had a nice cleanup-and-exit SIGTERM handler they they then uninstalled (because they no longer needed to do cleanup) and SIGTERM came as they were uninstalling their handler, we'd drop the SIGTERM on the floor and neither call their cleanup code nor have the signal perform its default action <desrt> so the program would receive a SIGTERM and never exit <desrt> i guess we need to re-raise the signal in these cases... <desrt> ...or not attempt to claim that we support this behaviour at all
I think a number of Gentoo consolekit users are being affected by this race: https://bugs.gentoo.org/show_bug.cgi?id=501330
Review of attachment 249829 [details] [review]: updating patch status to match reality
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/733.