Re: Another bug when suspending pipelines

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Fri, 16 Sep 2016 13:33:02 +0100
Peter Stephenson <p.stephenson@xxxxxxxxxxx> wrote:
> To check my code for the other pipeline suspending problem, I came up
> with a command designed to check different cases, in particular where
> the left of the pipeline was still running:
> 
>   (sleep 5; print foo) | { sleep 5; read bar; print $bar; }
> 
> Suspending this sometimes doesn't work: the ^Z is delayed and gets
> relayed to the parent shell.

I've had trouble getting this to happen again, so it looks like it's
quite an obscure race.  It's not necessarily directly related to
list_pipe's Wirkung, in either English, German or Old Norse.

> Sometimes even if it suspends, it stops again when you fg it, then
> the second fg allows it to complete.

This is much easier to reproduce, and it looks like it's related,
serendipitously, to the condition I fixed earlier today --- the forked
zsh can be stopped too many times because it's in the same process
groups as the other stuff.

I've now run out of reasons why it should be in the same process group
at all, so this extends the other fix until we find out why...

pws

diff --git a/Src/exec.c b/Src/exec.c
index 0755d55..9a7234e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1710,39 +1710,23 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 			break;
 		    }
 		    else {
-			Job pjn = jobtab + list_pipe_job;
 			close(synch[0]);
 			entersubsh(ESUB_ASYNC);
 			/*
 			 * At this point, we used to attach this process
 			 * to the process group of list_pipe_job (the
 			 * new superjob) any time that was still available.
-			 * That caused problems when the fork happened
-			 * early enough that the subjob is in that
-			 * process group, since then we will stop this
-			 * job when we signal the subjob, and we have
-			 * no way here to know that we shouldn't also
-			 * send STOP to itself, resulting in two stops.
-			 * So don't do that if the original
-			 * list_pipe_job has exited.
+			 * That caused problems in at least two
+			 * cases because this forked shell was then
+			 * suspended with the right hand side of the
+			 * pipeline, and the SIGSTOP below suspended
+			 * it a second time when it was continued.
 			 *
-			 * The choice here needs to match the assumption
-			 * made when handling SUBLEADER above that the
-			 * process group is our own PID.  I'm not sure
-			 * if there's a potential race for that.  But
-			 * setting up a new process group if the
-			 * superjob is still functioning seems the wrong
-			 * thing to do.
+			 * It's therefore not clear entirely why you'd ever
+			 * do anything other than the following, but no
+			 * doubt we'll find out...
 			 */
-			if (pjn->procs &&
-			    (pjn->stat & STAT_INUSE) &&
-			    !(pjn->stat & STAT_DONE)) {
-			    if (setpgrp(0L, mypgrp = jobtab[list_pipe_job].gleader)
-				== -1) {
-				setpgrp(0L, mypgrp = getpid());
-			    }
-			} else
-			    setpgrp(0L, mypgrp = getpid());
+			setpgrp(0L, mypgrp = getpid());
 			close(synch[1]);
 			kill(getpid(), SIGSTOP);
 			list_pipe = 0;