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

@@ -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;

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);

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) {
+	if (signo == SIGINT || signo == SIGQUIT)
-		breakwaitcmd = 1;
+		pendingsig_waitcmd = signo;
-		pendingsig = 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;

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;