Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: Bug related to stdin/always/jobcontrol



On Wed, 14 Sep 2016 18:31:05 +0100
Peter Stephenson <p.stephenson@xxxxxxxxxxx> wrote:
> I think I might be getting close with the attached patch... first
> guess is we need to create a new process group, or something.

The subjob needs to keep its process group because it's too late to
change it; I found out what was missing, it needs process group leader
assigning from that of list_pipe_job (obviously, right?)  Seems a bit
weird this never got noticed before, but it's hard to see how it can be
wrong since this is the way the pipeline always works, regardless of
whether the ls process or equivalent has exited.  I've tried a few bits
of job control with multiple different list-pipe-style constructs lying
around and they still seem to work.

Looks like the pgrp for the new superjob is irrelevant as it's kept out
of the way till the last minute, so no changes there.

I was shooting myself in the foot wth the change for "other".  It turns
out "other" means other job if this is the superjob, and other process
if this is the subjob (obviously, right?)  Je est veritablement un
autre.

I've made the claiming of the subjob safe by marking it as orphaned when
the original superjob, the ls process in the example, exits.  So if that
doesn't happen this doesn't kick in, so this shouldn't be stomping on
anything that already works.

I present the solution for verification.  If accepted, I will be
demanding a certifcate of some sort.

But there's still one thing I don't understand.  When
Scooby... er... sorry, force of habit... it's this chunk which is for
SIGCONT when we're dealing with a superjob in killjb():

                for (pn = jn->procs; pn->next; pn = pn->next)
                    if (kill(pn->pid, sig) == -1 && errno != ESRCH)
			err = -1;

The exit test is pn->next, so we skip the last process in the superjob,
which is a bit weird.  Certainly, changing that to the more obvious pn
causes the shell process acting as superjob in this case to be started
too early.  However, I don't know what happens in more normal cases.
I've left it alone.

pws

diff --git a/Src/exec.c b/Src/exec.c
index cfd633a..21270c8 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1570,6 +1570,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 
 	    if (nowait) {
 		if(!pline_level) {
+		    int jobsub;
 		    struct process *pn, *qn;
 
 		    curjob = newjob;
@@ -1582,6 +1583,20 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 		    if (!jn->procs->next || lpforked == 2) {
 			jn->gleader = list_pipe_pid;
 			jn->stat |= STAT_SUBLEADER;
+			/*
+			 * Pick up any subjob that's still lying around
+			 * as it's now our responsibility.
+			 * If we find it we're a SUPERJOB.
+			 */
+			for (jobsub = 1; jobsub <= maxjob; jobsub++) {
+			    Job jnsub = jobtab + jobsub;
+			    if (jnsub->stat & STAT_SUBJOB_ORPHANED) {
+				jn->other = jobsub;
+				jn->stat |= STAT_SUPERJOB;
+				jnsub->stat &= ~STAT_SUBJOB_ORPHANED;
+				jnsub->other = list_pipe_pid;
+			    }
+			}
 		    }
 		    for (pn = jobtab[jn->other].procs; pn; pn = pn->next)
 			if (WIFSTOPPED(pn->status))
@@ -1593,7 +1608,8 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 		    }
 
 		    jn->stat &= ~(STAT_DONE | STAT_NOPRINT);
-		    jn->stat |= STAT_STOPPED | STAT_CHANGED | STAT_LOCKED;
+		    jn->stat |= STAT_STOPPED | STAT_CHANGED | STAT_LOCKED |
+			STAT_INUSE;
 		    printjob(jn, !!isset(LONGLISTJOBS), 1);
 		}
 		else if (newjob != list_pipe_job)
@@ -1669,6 +1685,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 			    jobtab[list_pipe_job].stat |= STAT_SUPERJOB;
 			    jn->stat |= STAT_SUBJOB | STAT_NOPRINT;
 			    jn->other = pid;
+			    jn->gleader = jobtab[list_pipe_job].gleader;
 			}
 			if ((list_pipe || last1) && hasprocs(list_pipe_job))
 			    killpg(jobtab[list_pipe_job].gleader, SIGSTOP);
diff --git a/Src/jobs.c b/Src/jobs.c
index 6bc3616..9284c71 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -128,7 +128,7 @@ makerunning(Job jn)
     Process pn;
 
     jn->stat &= ~STAT_STOPPED;
-    for (pn = jn->procs; pn; pn = pn->next)
+    for (pn = jn->procs; pn; pn = pn->next) {
 #if 0
 	if (WIFSTOPPED(pn->status) && 
 	    (!(jn->stat & STAT_SUPERJOB) || pn->next))
@@ -136,6 +136,7 @@ makerunning(Job jn)
 #endif
         if (WIFSTOPPED(pn->status))
 	    pn->status = SP_RUNNING;
+    }
 
     if (jn->stat & STAT_SUPERJOB)
 	makerunning(jobtab + jn->other);
@@ -236,7 +237,7 @@ handle_sub(int job, int fg)
     if ((sj->stat & STAT_DONE) || (!sj->procs && !sj->auxprocs)) {
 	struct process *p;
 		    
-	for (p = sj->procs; p; p = p->next)
+	for (p = sj->procs; p; p = p->next) {
 	    if (WIFSIGNALED(p->status)) {
 		if (jn->gleader != mypgrp && jn->procs->next)
 		    killpg(jn->gleader, WTERMSIG(p->status));
@@ -246,6 +247,7 @@ handle_sub(int job, int fg)
 		kill(sj->other, WTERMSIG(p->status));
 		break;
 	    }
+	}
 	if (!p) {
 	    int cp;
 
@@ -1316,6 +1318,11 @@ deletejob(Job jn, int disowning)
 	attachtty(mypgrp);
 	adjustwinsize(0);
     }
+    if (jn->stat & STAT_SUPERJOB) {
+	Job jno = jobtab + jn->other;
+	if (jno->stat & STAT_SUBJOB)
+	    jno->stat |= STAT_SUBJOB_ORPHANED;
+    }
 
     freejob(jn, 1);
 }
diff --git a/Src/zsh.h b/Src/zsh.h
index 2dc5e7e..bb8ce13 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -983,7 +983,8 @@ struct jobfile {
 
 struct job {
     pid_t gleader;		/* process group leader of this job  */
-    pid_t other;		/* subjob id or subshell pid         */
+    pid_t other;		/* subjob id (SUPERJOB)
+				 * or subshell pid (SUBJOB) */
     int stat;                   /* see STATs below                   */
     char *pwd;			/* current working dir of shell when *
 				 * this job was spawned              */
@@ -1015,6 +1016,8 @@ struct job {
 #define STAT_SUBLEADER  (0x2000) /* is super-job, but leader is sub-shell */
 
 #define STAT_BUILTIN    (0x4000) /* job at tail of pipeline is a builtin */
+#define STAT_SUBJOB_ORPHANED (0x8000)
+                                 /* STAT_SUBJOB with STAT_SUPERJOB exited */
 
 #define SP_RUNNING -1		/* fake status for jobs currently running */
 



Messages sorted by: Reverse Date, Date, Thread, Author