From 3b5d3b7e1f320b0bfbe48024a586c0a22375aa2d Mon Sep 17 00:00:00 2001
From: Nils Philippsen <nils@redhat.com>
Date: Thu, 3 Jul 2014 10:38:03 +0200
Subject: [PATCH] patch: signal-handling
Squashed commit of the following:
commit 1e9e8cf5edc469114c8eadf46817cd5c1261b35c
Author: Nils Philippsen <nils@redhat.com>
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 <nils@redhat.com>
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 <sys/wait.h>
+#include <glib-unix.h>
#include <stdarg.h>
@@ -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