zsh-workers
 help / color / mirror / code / Atom feed
From: Peter Stephenson <p.stephenson@samsung.com>
To: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: "echo | ps -j $(:) | cat | cat | cat" runs components in different process groups
Date: Tue, 10 Apr 2018 10:53:30 +0100	[thread overview]
Message-ID: <20180410105330.704853ae@camnpupstephen.cam.scsc.local> (raw)
In-Reply-To: <180323163614.ZM10192@torch.brasslantern.com>

On Fri, 23 Mar 2018 16:36:14 -0700
Bart Schaefer <schaefer@brasslantern.com> 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


  parent reply	other threads:[~2018-04-10  9:53 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-23 16:16 Stephane Chazelas
2018-03-23 23:36 ` Bart Schaefer
2018-03-24  5:19   ` Bart Schaefer
2018-03-24  8:05     ` Joey Pabalinas
2018-03-24 17:34       ` Stephane Chazelas
2018-03-24 22:23         ` Bart Schaefer
2018-03-25  7:00           ` Stephane Chazelas
2018-03-25  8:26             ` Stephane Chazelas
2018-03-25  7:48           ` Stephane Chazelas
2018-03-25 18:09             ` Stephane Chazelas
2018-03-27 10:17         ` Stephane Chazelas
2018-03-24 22:09       ` Bart Schaefer
2018-04-10 11:45         ` Peter Stephenson
2018-04-10 13:59           ` Peter Stephenson
2018-04-11 22:10             ` Bart Schaefer
2018-04-12 16:23               ` Peter Stephenson
2018-04-15 16:23                 ` Stephane Chazelas
2018-04-15 17:38                   ` Bart Schaefer
2018-04-15 18:58                     ` Stephane Chazelas
2018-04-17  5:39                       ` Bart Schaefer
2018-04-17  9:19                         ` Peter Stephenson
2018-04-17 16:09                           ` Bart Schaefer
2018-04-17 16:27                             ` Bart Schaefer
2018-04-17 16:35                             ` Peter Stephenson
2018-04-17 17:52                               ` Bart Schaefer
2018-04-19  9:40                                 ` Peter Stephenson
2018-04-20  9:28                                   ` Forking earlier for background code Peter Stephenson
2018-04-20 10:01                                     ` Peter Stephenson
2018-04-20 12:54                                       ` Vin Shelton
     [not found]                                         ` <CGME20180420132008eucas1p1c297624e870975cd892c74254970faab@eucas1p1.samsung.com>
2018-04-20 13:20                                           ` Peter Stephenson
2018-04-20 16:01                                             ` Vin Shelton
2018-04-23  8:29                                     ` Peter Stephenson
2018-05-01  9:23                                       ` Peter Stephenson
     [not found]                                   ` <F3A62E38-24E2-4A62-8E19-F54C9B81E9E5@kba.biglobe.ne.jp>
2018-04-23 13:52                                     ` "echo | ps -j $(:) | cat | cat | cat" runs components in different process groups Peter Stephenson
2018-04-23 14:03                                       ` Peter Stephenson
     [not found]                                         ` <CGME20180423140859eucas1p2591bf1422614209979d4890383268c37@eucas1p2.samsung.com>
2018-04-23 14:08                                           ` Peter Stephenson
2018-04-23 15:29                                         ` Jun T.
2018-04-18  6:29                           ` Stephane Chazelas
2018-04-18 16:33                             ` Bart Schaefer
2018-04-10  9:53   ` Peter Stephenson [this message]
2018-03-25  7:38 ` Stephane Chazelas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180410105330.704853ae@camnpupstephen.cam.scsc.local \
    --to=p.stephenson@samsung.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).