[xsane] fix signal handling (#1073698)
Nils Philippsen
nphilipp at fedoraproject.org
Wed Mar 19 13:33:59 UTC 2014
commit 59a0b0a864f68d6b025ee69d4dcf0b9a4b6da031
Author: Nils Philippsen <nils at redhat.com>
Date: Wed Mar 19 14:32:01 2014 +0100
fix signal handling (#1073698)
xsane-0.999-signal-handling.patch | 220 +++++++++++++++++++++++++++++++++++++
xsane.spec | 7 +
2 files changed, 227 insertions(+), 0 deletions(-)
---
diff --git a/xsane-0.999-signal-handling.patch b/xsane-0.999-signal-handling.patch
new file mode 100644
index 0000000..d827217
--- /dev/null
+++ b/xsane-0.999-signal-handling.patch
@@ -0,0 +1,220 @@
+From 54a700c45c81f1ffff5f3dfcecb3929143e31d48 Mon Sep 17 00:00:00 2001
+From: Nils Philippsen <nils at redhat.com>
+Date: Wed, 19 Mar 2014 13:23:16 +0100
+Subject: [PATCH] patch: signal-handling
+
+Squashed commit of the following:
+
+commit 2b629e1d9c60dcf26defe6d05b515611874db270
+Author: Nils Philippsen <nils at 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 use of non-reentrant functions safe.
+---
+ src/xsane.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
+ 1 file changed, 131 insertions(+), 15 deletions(-)
+
+diff --git a/src/xsane.c b/src/xsane.c
+index eee76ff..02b4da4 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(gint fd,
++ 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(gint fd,
++ 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
++ * EGAIN/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;
+@@ -6062,18 +6170,26 @@ void xsane_interface(int argc, char **argv)
+ }
+ }
+
++ if (!g_unix_open_pipe(xsane_signal_pipe, FD_CLOEXEC, NULL) ||
++ fcntl(xsane_signal_pipe[0], F_SETFL, O_NONBLOCK, 1) ||
++ fcntl(xsane_signal_pipe[1], F_SETFL, O_NONBLOCK, 1) ||
++ !g_unix_fd_add(xsane_signal_pipe[0], G_IO_IN | 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.8.5.3
+
diff --git a/xsane.spec b/xsane.spec
index 3e8a1ee..402ecdd 100644
--- a/xsane.spec
+++ b/xsane.spec
@@ -61,6 +61,9 @@ Patch11: xsane-0.999-pdf-no-high-bpp.patch
# build against lcms 2.x
# submitted to upstream (Oliver Rauch) via email, 2013-09-23
Patch12: xsane-0.999-lcms2.patch
+# fix signal handling (#1073698)
+# submitted to upstream (Oliver Rauch) via email, 2014-03-19
+Patch13: xsane-0.999-signal-handling.patch
# autoconf-generated files
Patch100: xsane-0.999-7-autoconf.patch.bz2
License: GPLv2+
@@ -126,6 +129,7 @@ done
%patch10 -p1 -b .man-page
%patch11 -p1 -b .pdf-no-high-bpp
%patch12 -p1 -b .lcms2
+%patch13 -p1 -b .signal-handling
%patch100 -p1 -b .autoconf
@@ -230,6 +234,9 @@ fi
%{_datadir}/sane/xsane
%changelog
+* Wed Mar 19 2014 Nils Philippsen <nils at redhat.com>
+- fix signal handling (#1073698)
+
* Mon Sep 23 2013 Nils Philippsen <nils at redhat.com> - 0.999-7
- get rid of ancient compat cruft
- build against lcms2
More information about the scm-commits
mailing list