Author: jilles
Date: Mon Sep 2 21:57:46 2013
New Revision: 255157
URL: http://svnweb.freebsd.org/changeset/base/255157
Log:
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.
Modified:
head/bin/sh/jobs.c
head/bin/sh/jobs.h
head/bin/sh/trap.c
head/bin/sh/trap.h
Modified: head/bin/sh/jobs.c
==============================================================================
--- head/bin/sh/jobs.c Mon Sep 2 20:44:19 2013 (r255156)
+++ head/bin/sh/jobs.c Mon Sep 2 21:57:46 2013 (r255157)
@@ -83,13 +83,12 @@ static struct job *bgjob = NULL; /* last
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 _
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 *origstat
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;
Modified: head/bin/sh/jobs.h
==============================================================================
--- head/bin/sh/jobs.h Mon Sep 2 20:44:19 2013 (r255156)
+++ head/bin/sh/jobs.h Mon Sep 2 21:57:46 2013 (r255157)
@@ -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);
Modified: head/bin/sh/trap.c
==============================================================================
--- head/bin/sh/trap.c Mon Sep 2 20:44:19 2013 (r255156)
+++ head/bin/sh/trap.c Mon Sep 2 21:57:46 2013 (r255157)
@@ -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;
Modified: head/bin/sh/trap.h
==============================================================================
--- head/bin/sh/trap.h Mon Sep 2 20:44:19 2013 (r255156)
+++ head/bin/sh/trap.h Mon Sep 2 21:57:46 2013 (r255157)
@@ -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;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
