From ae84eb37bd6a1df11d171b8fbdf02fa57c628fbf Mon Sep 17 00:00:00 2001 From: jilles Date: Mon, 2 Sep 2013 21:57:46 +0000 Subject: sh: Fix race condition with signals and wait or set -T. The change in r238888 was incomplete. It was still possible for a trapped signal to arrive before the shell went to sleep (sigsuspend()) because a check was missing or because the signal arrived before in_waitcmd was set. On SMP, this bug sometimes caused the builtins/wait4.0 test to take 1 second to execute; it then might or might not fail. On UP, the test almost always failed. --- bin/sh/jobs.c | 32 +++++++++++++++----------------- bin/sh/jobs.h | 2 -- bin/sh/trap.c | 16 ++++------------ bin/sh/trap.h | 1 + 4 files changed, 20 insertions(+), 31 deletions(-) (limited to 'bin') diff --git a/bin/sh/jobs.c b/bin/sh/jobs.c index b6ea4da..bbb954a 100644 --- a/bin/sh/jobs.c +++ b/bin/sh/jobs.c @@ -83,13 +83,12 @@ static struct job *bgjob = NULL; /* last background process */ static struct job *jobmru; /* most recently used job list */ static pid_t initialpgrp; /* pgrp of shell on invocation */ #endif -int in_waitcmd = 0; /* are we in waitcmd()? */ -volatile sig_atomic_t breakwaitcmd = 0; /* should wait be terminated? */ static int ttyfd = -1; /* mode flags for dowait */ #define DOWAIT_BLOCK 0x1 /* wait until a child exits */ -#define DOWAIT_SIG 0x2 /* if DOWAIT_BLOCK, abort on signals */ +#define DOWAIT_SIG 0x2 /* if DOWAIT_BLOCK, abort on SIGINT/SIGQUIT */ +#define DOWAIT_SIG_ANY 0x4 /* if DOWAIT_SIG, abort on any signal */ #if JOBS static void restartjob(struct job *); @@ -484,7 +483,7 @@ waitcmd(int argc __unused, char **argv __unused) static int waitcmdloop(struct job *job) { - int status, retval; + int status, retval, sig; struct job *jp; /* @@ -492,7 +491,6 @@ waitcmdloop(struct job *job) * received. */ - in_waitcmd++; do { if (job != NULL) { if (job->state == JOBDONE) { @@ -508,7 +506,6 @@ waitcmdloop(struct job *job) if (job == bgjob) bgjob = NULL; } - in_waitcmd--; return retval; } } else { @@ -524,7 +521,6 @@ waitcmdloop(struct job *job) } for (jp = jobtab ; ; jp++) { if (jp >= jobtab + njobs) { /* no running procs */ - in_waitcmd--; return 0; } if (jp->used && jp->state == 0) @@ -532,9 +528,10 @@ waitcmdloop(struct job *job) } } } while (dowait(DOWAIT_BLOCK | DOWAIT_SIG, (struct job *)NULL) != -1); - in_waitcmd--; - return pendingsig + 128; + sig = pendingsig_waitcmd; + pendingsig_waitcmd = 0; + return sig + 128; } @@ -990,7 +987,8 @@ waitforjob(struct job *jp, int *origstatus) INTOFF; TRACE(("waitforjob(%%%td) called\n", jp - jobtab + 1)); while (jp->state == 0) - if (dowait(DOWAIT_BLOCK | (Tflag ? DOWAIT_SIG : 0), jp) == -1) + if (dowait(DOWAIT_BLOCK | (Tflag ? DOWAIT_SIG | + DOWAIT_SIG_ANY : 0), jp) == -1) dotrap(); #if JOBS if (jp->jobctl) { @@ -1081,12 +1079,17 @@ dowait(int mode, struct job *job) pid = wait3(&status, wflags, (struct rusage *)NULL); TRACE(("wait returns %d, status=%d\n", (int)pid, status)); if (pid == 0 && (mode & DOWAIT_SIG) != 0) { - sigsuspend(&omask); pid = -1; + if (((mode & DOWAIT_SIG_ANY) != 0 ? + pendingsig : pendingsig_waitcmd) != 0) { + errno = EINTR; + break; + } + sigsuspend(&omask); if (int_pending()) break; } - } while (pid == -1 && errno == EINTR && breakwaitcmd == 0); + } while (pid == -1 && errno == EINTR); if (pid == -1 && errno == ECHILD && job != NULL) job->state = JOBDONE; if ((mode & DOWAIT_SIG) != 0) { @@ -1095,11 +1098,6 @@ dowait(int mode, struct job *job) sigprocmask(SIG_SETMASK, &omask, NULL); INTON; } - if (breakwaitcmd != 0) { - breakwaitcmd = 0; - if (pid <= 0) - return -1; - } if (pid <= 0) return pid; INTOFF; diff --git a/bin/sh/jobs.h b/bin/sh/jobs.h index 1570b46..892f129 100644 --- a/bin/sh/jobs.h +++ b/bin/sh/jobs.h @@ -83,8 +83,6 @@ enum { }; extern int job_warning; /* user was warned about stopped jobs */ -extern int in_waitcmd; /* are we in waitcmd()? */ -extern volatile sig_atomic_t breakwaitcmd; /* break wait to process traps? */ void setjobctl(int); void showjobs(int, int); diff --git a/bin/sh/trap.c b/bin/sh/trap.c index 1411289..3fc8566 100644 --- a/bin/sh/trap.c +++ b/bin/sh/trap.c @@ -74,6 +74,7 @@ __FBSDID("$FreeBSD$"); static char sigmode[NSIG]; /* current value of signal */ volatile sig_atomic_t pendingsig; /* indicates some signal received */ +volatile sig_atomic_t pendingsig_waitcmd; /* indicates SIGINT/SIGQUIT received */ int in_dotrap; /* do we execute in a trap handler? */ static char *volatile trap[NSIG]; /* trap handler commands */ static volatile sig_atomic_t gotsig[NSIG]; @@ -389,23 +390,13 @@ onsig(int signo) } /* If we are currently in a wait builtin, prepare to break it */ - if ((signo == SIGINT || signo == SIGQUIT) && in_waitcmd != 0) { - breakwaitcmd = 1; - pendingsig = signo; - } + if (signo == SIGINT || signo == SIGQUIT) + pendingsig_waitcmd = signo; if (trap[signo] != NULL && trap[signo][0] != '\0' && (signo != SIGCHLD || !ignore_sigchld)) { gotsig[signo] = 1; pendingsig = signo; - - /* - * If a trap is set, not ignored and not the null command, we - * need to make sure traps are executed even when a child - * blocks signals. - */ - if (Tflag && !(trap[signo][0] == ':' && trap[signo][1] == '\0')) - breakwaitcmd = 1; } #ifndef NO_HISTORY @@ -428,6 +419,7 @@ dotrap(void) in_dotrap++; for (;;) { pendingsig = 0; + pendingsig_waitcmd = 0; for (i = 1; i < NSIG; i++) { if (gotsig[i]) { gotsig[i] = 0; diff --git a/bin/sh/trap.h b/bin/sh/trap.h index 0a05d8d..a962251 100644 --- a/bin/sh/trap.h +++ b/bin/sh/trap.h @@ -34,6 +34,7 @@ */ extern volatile sig_atomic_t pendingsig; +extern volatile sig_atomic_t pendingsig_waitcmd; extern int in_dotrap; extern volatile sig_atomic_t gotwinch; -- cgit v1.1