From 4e2d549e8b2e34c5bd4c77a4a6eba99ed0912168 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 3 Feb 2011 20:40:47 +0100 Subject: Remove a race condition in SIGCHLD handler. If the SIGCHLD is delivered *just* as we're installing our handler, there's a race condition in the setting of the old handler. So instead rely on the OS to set it properly before it calls our handler. Reviewed-by: Olivier Goffart --- src/corelib/io/qprocess_unix.cpp | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index ba61bda..b93fce9 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -120,7 +120,7 @@ static inline char *strdup(const char *data) #endif static int qt_qprocess_deadChild_pipe[2]; -static void (*qt_sa_old_sigchld_handler)(int) = 0; +static struct sigaction qt_sa_old_sigchld_handler; static void qt_sa_sigchld_handler(int signum) { qt_safe_write(qt_qprocess_deadChild_pipe[1], "", 1); @@ -128,8 +128,10 @@ static void qt_sa_sigchld_handler(int signum) fprintf(stderr, "*** SIGCHLD\n"); #endif - if (qt_sa_old_sigchld_handler && qt_sa_old_sigchld_handler != SIG_IGN) - qt_sa_old_sigchld_handler(signum); + // load it as volatile + void (*oldAction)(int) = ((volatile struct sigaction *)&qt_sa_old_sigchld_handler)->sa_handler; + if (oldAction && oldAction != SIG_IGN) + oldAction(signum); } static inline void add_fd(int &nfds, int fd, fd_set *fdset) @@ -190,14 +192,11 @@ QProcessManager::QProcessManager() // set up the SIGCHLD handler, which writes a single byte to the dead // child pipe every time a child dies. - struct sigaction oldAction; struct sigaction action; memset(&action, 0, sizeof(action)); action.sa_handler = qt_sa_sigchld_handler; action.sa_flags = SA_NOCLDSTOP; - ::sigaction(SIGCHLD, &action, &oldAction); - if (oldAction.sa_handler != qt_sa_sigchld_handler) - qt_sa_old_sigchld_handler = oldAction.sa_handler; + ::sigaction(SIGCHLD, &action, &qt_sa_old_sigchld_handler); } QProcessManager::~QProcessManager() @@ -217,14 +216,10 @@ QProcessManager::~QProcessManager() qDeleteAll(children.values()); children.clear(); - struct sigaction oldAction; - struct sigaction action; - memset(&action, 0, sizeof(action)); - action.sa_handler = qt_sa_old_sigchld_handler; - action.sa_flags = SA_NOCLDSTOP; - ::sigaction(SIGCHLD, &action, &oldAction); - if (oldAction.sa_handler != qt_sa_sigchld_handler) { - ::sigaction(SIGCHLD, &oldAction, 0); + struct sigaction currentAction; + ::sigaction(SIGCHLD, 0, ¤tAction); + if (currentAction.sa_handler == qt_sa_sigchld_handler) { + ::sigaction(SIGCHLD, &qt_sa_old_sigchld_handler, 0); } } -- cgit v0.12