Re: "echo | ps -j $(:) | cat | cat | cat" runs components in different process groups

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Fri, 23 Mar 2018 16:36:14 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Mar 23,  4:16pm, Stephane Chazelas wrote:
> }
> } $ echo | ps -j $(:) | cat | cat | cat
> }   PID  PGID   SID TTY          TIME CMD
> }  4858  4858  4858 pts/4    00:00:02 zsh
> } 23813 23813  4858 pts/4    00:00:00 zsh
> } 23895 23895  4858 pts/4    00:00:00 ps
> } 23896 23896  4858 pts/4    00:00:00 cat
> } 23897 23897  4858 pts/4    00:00:00 cat
> } 23898 23898  4858 pts/4    00:00:00 cat
> } 
> } See how they all run in different process groups (they should
> } all be in the same one, same job).
>...
> Zsh wants to use "echo" as the group leader because it was the first
> to fork, but because $(:) delayed the execution of "ps" the "echo"
> process has already exited and cannot be made group leader.  As the
> fallback the other processes each get made into its own process group.
>...
> Otherwise we'd have to notice that the group leader is gone and change
> to the second forked process, and so on, until we found a leader that
> would stick for the remainder of the forks.

The following patch gives me the same with and without the $(:).
Changing gleader ASAP seems the only sane thing to do --- else we end up
in a maze of forks and none of the subshells know what's going on.

But I'm a bit worried that we're going to end up with something
following the original leader and something not?  If that did happen,
does it mean the pgrp is still valid, and if so can we test that without
actually changing it?  Or does it mean there's no zsh-specific problem
because those earlier processes are pgrp orphans in any case?

As the setpgrp() happens in subprocess I don't think there's any future
in recording what processes have set the process group already.  Surely
this must be a "well-known problem" with a "well-known" solution?

I'm not convinced Joey's case is actually (directly) related to this (or
rather, to anything fixed by this or Bart's change --- it's obviously
somewhere in the same area).

% echo | ps -j $(:) | cat | cat | cat
  PID  PGID   SID TTY          TIME CMD
18367 18367 18367 pts/4    00:00:07 zsh
20179 20179 18367 pts/4    00:00:02 zsh
20199 20199 18367 pts/4    00:00:00 ps
20200 20199 18367 pts/4    00:00:00 cat
20201 20199 18367 pts/4    00:00:00 cat
20202 20199 18367 pts/4    00:00:00 cat
% echo | ps -j | cat | cat | cat
  PID  PGID   SID TTY          TIME CMD
18367 18367 18367 pts/4    00:00:07 zsh
20179 20179 18367 pts/4    00:00:02 zsh
20204 20203 18367 pts/4    00:00:00 ps
20205 20203 18367 pts/4    00:00:00 cat
20206 20203 18367 pts/4    00:00:00 cat
20207 20203 18367 pts/4    00:00:00 cat

diff --git a/Src/signals.c b/Src/signals.c
index 94f379e..5917683 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -537,6 +537,18 @@ wait_for_processes(void)
 #else
 		update_process(pn, status);
 #endif
+		if (pn->pid == jn->gleader) {
+		    Process pnn = pn->next;
+		    while (pnn) {
+			if (pnn->status == SP_RUNNING) {
+			    jn->gleader = pnn->pid;
+			    break;
+			}
+			pnn = pnn->next;
+		    }
+		    if (pn->pid == jn->gleader)
+			jn->gleader = 0;
+		}
 	    }
 	    update_job(jn);
 	} else if (findproc(pid, &jn, &pn, 1)) {

pws