43535: Fixes for bg / fg handling of superjobs.

Be more consistent about marking both superjob and subjob as
running when sending SIGCONT.

Send SIGCONT to superjob / subjob combination any time it is put
in foreground, even if thought running, since subjob may invisibly
have suspended.

When waiting for superjob, wait for subjob, too.

3 files changed, 54 insertions, 19 deletions

diff --git a/ChangeLog b/ChangeLog
index 8a309c42a..c61e376e2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2018-09-24  Peter Stephenson  <p.w.stephenson@ntlworld.com>
+
+	* 43535: Src/jobs.c, Src/signals.c: fixes for bg / fg handling
+	of superjobs.  Be more consistent about marking jobs running;
+	always send SIGCONT when putting superjob / subjob combination
+	into foreground; wait for both superjob and subjob when waiting
+	for superjob.
+
 2018-09-24  Daniel Shahaf  <danielsh@apache.org>
 
 	* 43493: Test/V07pcre.ztst: Have V07pcre fail if PCRE was enabled
diff --git a/Src/jobs.c b/Src/jobs.c
index 2d58319a8..c399f1d3e 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1569,10 +1569,8 @@ zwaitjob(int job, int wait_cmd)
 
            errflag = 0; */
 
-	    if (subsh) {
+	    if (subsh)
 		killjb(jn, SIGCONT);
-		jn->stat &= ~STAT_STOPPED;
-	    }
 	    if (jn->stat & STAT_SUPERJOB)
 		if (handle_sub(jn - jobtab, 1))
 		    break;
@@ -1590,6 +1588,17 @@ zwaitjob(int job, int wait_cmd)
     return 0;
 }
 
+static void waitonejob(Job jn)
+{
+    if (jn->procs || jn->auxprocs)
+	zwaitjob(jn - jobtab, 0);
+    else {
+	deletejob(jn, 0);
+	pipestats[0] = lastval;
+	numpipestats = 1;
+    }
+}
+
 /* wait for running job to finish */
 
 /**/
@@ -1599,13 +1608,10 @@ waitjobs(void)
     Job jn = jobtab + thisjob;
     DPUTS(thisjob == -1, "No valid job in waitjobs.");
 
-    if (jn->procs || jn->auxprocs)
-	zwaitjob(thisjob, 0);
-    else {
-	deletejob(jn, 0);
-	pipestats[0] = lastval;
-	numpipestats = 1;
-    }
+    waitonejob(jn);
+    if (jn->stat & STAT_SUPERJOB)
+	waitonejob(jobtab + jn->other);
+	
     thisjob = -1;
 }
 
@@ -2294,11 +2300,8 @@ bin_fg(char *name, char **argv, Options ops, int func)
 	    Process p;
 
 	    if (findproc(pid, &j, &p, 0)) {
-		if (j->stat & STAT_STOPPED) {
+		if (j->stat & STAT_STOPPED)
 		    retval = (killjb(j, SIGCONT) != 0);
-		    if (retval == 0)
-			makerunning(j);
-		}
 		if (retval == 0) {
 		    /*
 		     * returns 0 for normal exit, else signal+128
@@ -2404,9 +2407,17 @@ bin_fg(char *name, char **argv, Options ops, int func)
 			((!jobtab[job].procs->next ||
 			  (jobtab[job].stat & STAT_SUBLEADER) ||
 			  killpg(jobtab[job].gleader, 0) == -1)) &&
-			jobtab[jobtab[job].other].gleader)
+			jobtab[jobtab[job].other].gleader) {
 			attachtty(jobtab[jobtab[job].other].gleader);
-		    else
+			/*
+			 * In case stopped by putting in background.
+			 * Usually that's visible to the user, who
+			 * can restart, but with a superjob this is done
+			 * behind the scenes, so do it here.  Should
+			 * be harmless if not needed.
+			 */
+			stopped = 1;
+		    } else
 			attachtty(jobtab[job].gleader);
 		}
 	    }
diff --git a/Src/signals.c b/Src/signals.c
index 26d88abc2..f294049c2 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -782,7 +782,20 @@ killjb(Job jn, int sig)
 		    if (kill(pn->pid, sig) == -1 && errno != ESRCH)
 			err = -1;
 
-                return err;
+		/*
+		 * The following marks both the superjob and subjob
+		 * as running, as done elsewhere.
+		 *
+		 * It's not entirely clear to me what the right way
+		 * to flag the status of a still-pausd final process,
+		 * as handled above, but we should be cnsistent about
+		 * this inside makerunning() rather than doing anything
+		 * special here.
+		 */
+		if (err != -1)
+		    makerunning(jn);
+
+		return err;
             }
             if (killpg(jobtab[jn->other].gleader, sig) == -1 && errno != ESRCH)
 		err = -1;
@@ -792,8 +805,11 @@ killjb(Job jn, int sig)
 
 	    return err;
         }
-        else
-	    return killpg(jn->gleader, sig);
+        else {
+	    err = killpg(jn->gleader, sig);
+	    if (sig == SIGCONT && err != -1)
+		makerunning(jn);
+	}
     }
     for (pn = jn->procs; pn; pn = pn->next) {
 	/*