From 3b5d3b7e1f320b0bfbe48024a586c0a22375aa2d Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Thu, 3 Jul 2014 10:38:03 +0200 Subject: [PATCH] patch: signal-handling Squashed commit of the following: commit 1e9e8cf5edc469114c8eadf46817cd5c1261b35c Author: Nils Philippsen Date: Thu Jul 3 10:14:52 2014 +0200 don't use g_unix_open_pipe(), g_unix_fd_add() These functions have only recently been added to glib. Use pipe()/ fcntl() and g_io_channel_unix_new()/g_io_add_watch() instead which are available in the minimum glib version needed for gtk+-2.x. commit acbdf3f693d3d2a78ee7490ca1bf76957daf00cf Author: Nils Philippsen Date: Thu Mar 13 13:38:12 2014 +0100 separate signal handlers in top and bottom half This is to avoid race-conditions occurring when a signal is received while the signal handler is not yet finished. It also avoids calling non-reentrant functions from a signal handler. The top half (the real signal handler) just writes a character into a pipe which gets picked up and serviced by the bottom half from the normal event loop, this serializes things and makes using non-reentrant functions safe. --- src/xsane.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 136 insertions(+), 15 deletions(-) diff --git a/src/xsane.c b/src/xsane.c index 2b9211b..fc2ebbe 100644 --- a/src/xsane.c +++ b/src/xsane.c @@ -47,6 +47,7 @@ #endif #include +#include #include @@ -121,6 +122,7 @@ static const Preferences_medium_t pref_default_medium[]= int DBG_LEVEL = 0; static guint xsane_resolution_timer = 0; +static int xsane_signal_pipe[2]; /* ---------------------------------------------------------------------------------------------------------------------- */ @@ -161,8 +163,11 @@ void xsane_pref_save(void); static int xsane_pref_restore(void); static void xsane_pref_save_media(void); static void xsane_pref_restore_media(void); -static RETSIGTYPE xsane_quit_handler(int signal); -static RETSIGTYPE xsane_sigchld_handler(int signal); +static RETSIGTYPE xsane_signal_handler_top_half(int signal); +static gboolean xsane_signal_handler_bottom_half(GIOChannel *source, + GIOCondition condition, + gpointer user_data); +static void xsane_sigchld_handler(void); static void xsane_quit(void); static void xsane_exit(void); static gint xsane_standard_option_win_delete(GtkWidget *widget, gpointer data); @@ -2296,16 +2301,119 @@ static void xsane_pref_restore_media(void) /* ---------------------------------------------------------------------------------------------------------------------- */ -static RETSIGTYPE xsane_quit_handler(int signal) +static RETSIGTYPE xsane_signal_handler_top_half(int signal) { - DBG(DBG_proc, "xsane_quit_handler\n"); + const char *msg_func = "xsane_signal_handler_top_half(): "; + const char *msg_short_write = "Short write() while processing signal.\n"; + const char *msg_err = "Error during write().\n"; + char sig_char; + ssize_t written; + int errno_saved = errno; - xsane_quit(); + switch (signal) + { + case SIGTERM: + sig_char = 't'; + break; + case SIGINT: + sig_char = 'i'; + break; + case SIGHUP: + sig_char = 'h'; + break; + case SIGCHLD: + sig_char = 'c'; + break; + default: + sig_char = '?'; + break; + } + + if ((written = write(xsane_signal_pipe[1], &sig_char, 1)) <= 0) + { + /* At this point, all bets are off. Salvage what we can. */ + + const char *msg = (written == 0) ? msg_short_write : msg_err; + + if ((write(STDERR_FILENO, msg_func, strlen(msg_func)) < 0) || + (write(STDERR_FILENO, msg, strlen(msg)) < 0)) + { + /* This is really a no-op, but at this point it doesn't really matter + * anymore if the writes succeeded or not. */ + goto bail_out; + } + +bail_out: + /* Ignore SIGCHLD errors, zombie processes don't hurt that much. */ + if (signal != SIGCHLD) + { + struct SIGACTION act; + memset(&act, 0, sizeof(act)); + act.sa_handler = SIG_DFL; + sigaction(signal, &act, NULL); + raise(signal); + } + } + + errno = errno_saved; +} + +static gboolean xsane_signal_handler_bottom_half(GIOChannel *source, + GIOCondition condition, + gpointer user_data) +{ + char sig_char; + ssize_t readlen; + + DBG(DBG_proc, "xsane_signal_handler_bottom_half\n"); + + while ((readlen = read(xsane_signal_pipe[0], &sig_char, 1)) != 0) + { + if (readlen < 0) + { + if (errno == EINTR) + { + /* if interrupted by signal, just repeat reading */ + continue; + } + else + { + break; + } + } + + switch (sig_char) + { + case 't': + case 'i': + case 'h': + xsane_quit(); + break; + case 'c': + xsane_sigchld_handler(); + break; + default: + DBG(DBG_error, + "Don't know how to cope with character-encoded signal: '%c'\n", + sig_char); + break; + } + } + + /* previous invocation might have read more than it should, so ignore + * EAGAIN/EWOULDBLOCK */ + if (readlen < 0 && errno != EAGAIN && errno != EWOULDBLOCK) + { + DBG(DBG_error, "Error while reading from pipe: %d '%s'\n", errno, + strerror(errno)); + } + + return TRUE; } /* ---------------------------------------------------------------------------------------------------------------------- */ -static RETSIGTYPE xsane_sigchld_handler(int signal) +static void xsane_sigchld_handler(void) { int status; XsaneChildprocess **childprocess_listptr = &xsane.childprocess_list; @@ -6026,6 +6134,8 @@ void xsane_interface(int argc, char **argv) { struct SIGACTION act; + GIOChannel *gio_pipe_read; + DBG(DBG_proc, "xsane_interface\n"); xsane.info_label = NULL; @@ -6069,18 +6179,29 @@ void xsane_interface(int argc, char **argv) } } + if ((pipe(xsane_signal_pipe) == -1) || + (fcntl(xsane_signal_pipe[0], F_SETFD, FD_CLOEXEC) == -1) || + (fcntl(xsane_signal_pipe[0], F_SETFL, O_NONBLOCK) == -1) || + (fcntl(xsane_signal_pipe[1], F_SETFD, FD_CLOEXEC) == -1) || + (fcntl(xsane_signal_pipe[1], F_SETFL, O_NONBLOCK) == -1) || + !(gio_pipe_read = g_io_channel_unix_new(xsane_signal_pipe[0])) || + !g_io_add_watch(gio_pipe_read, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI, + xsane_signal_handler_bottom_half, NULL)) + { + DBG(DBG_error, + "Couldn't create signal handling pipe, set flags on it or install\n" + "bottom half of handler.\n"); + exit(1); + } + /* define SIGTERM, SIGINT, SIGHUP-handler to make sure that e.g. all temporary files are deleted */ /* when xsane gets such a signal */ memset(&act, 0, sizeof(act)); - act.sa_handler = xsane_quit_handler; - sigaction(SIGTERM, &act, 0); - sigaction(SIGINT, &act, 0); - sigaction(SIGHUP, &act, 0); - - /* add a signal handler that cleans up zombie child processes */ - memset(&act, 0, sizeof(act)); - act.sa_handler = xsane_sigchld_handler; - sigaction(SIGCHLD, &act, 0); + act.sa_handler = xsane_signal_handler_top_half; + sigaction(SIGTERM, &act, NULL); + sigaction(SIGINT, &act, NULL); + sigaction(SIGHUP, &act, NULL); + sigaction(SIGCHLD, &act, NULL); gtk_main(); sane_exit(); -- 1.9.3