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 14:59:26 +0100 [thread overview]
Message-ID: <20180410145926.64c4f671@camnpupstephen.cam.scsc.local> (raw)
In-Reply-To: <20180410124545.13fccd5d@camnpupstephen.cam.scsc.local>
On Tue, 10 Apr 2018 12:45:45 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Sat, 24 Mar 2018 15:09:44 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Mar 23, 10:05pm, Joey Pabalinas wrote:
> > }
> > } > It could be I just haven't constructed the right test.
> > }
> > } It's kind of an *incredibly* unlikely edge-case but FWIW:
> > }
> > } > $ echo | ps -j $(: $(cat)) | cat | cat | cat
> > }
> > } The shell _seems_ to completely lock up after applying your patch;
> > on } my zsh 5.4.2 release version there are no problems (aside from
> > the } original process group leader issues).
>
>... [Stuff about the process group aspects I'm not discussing here] ...
I think all this still applies, but I've nothing new to add, so I've
edited it down to keep the messages manageable.
> But looking at ps output while ^C is failing to have any effect
> suggests things are worse than that. I think at this point the
> parent shell hasn't actually reaped the echo (it's <defunct>), so
> hasn't executed this chunk of code. That's got something to do with
> the fact that it's got child reaping blocked while trying to read
> output from the the $(: ...) process --- it's executing the same code
> as the forked shell waiting for the cat. However, even simple-minded
> child unblocking didn't seem to cause the zombie to go away, at which
> point I got stuck.
OK, the following change --- also too simple for the real world, so
treat as proof of concept --- *does* make this case interruptible. I
was forgetting that child_unblock() wasn't enough, you need to ensure
signals aren't queued.
So I suppose we need a way of making this safe(r)...
pws
diff --git a/Src/exec.c b/Src/exec.c
index e154d12..c22d37d 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4543,11 +4543,15 @@ getoutput(char *cmd, int qt)
return NULL;
} else if (pid) {
LinkList retval;
+ int q = queue_signal_level();
zclose(pipes[1]);
+ dont_queue_signals();
+ child_unblock();
retval = readoutput(pipes[0], qt, NULL);
fdtable[pipes[0]] = FDT_UNUSED;
waitforpid(pid, 0); /* unblocks */
+ restore_queue_signals(q);
lastval = cmdoutval;
return retval;
}
diff --git a/Src/signals.c b/Src/signals.c
index 94f379e..f86ae54 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -537,6 +537,11 @@ wait_for_processes(void)
#else
update_process(pn, status);
#endif
+ if (pn->pid == jn->gleader) {
+ jn->gleader = 0;
+ /* HERE: check this was actually in foreground... */
+ attachtty(mypgrp);
+ }
}
update_job(jn);
} else if (findproc(pid, &jn, &pn, 1)) {
next prev parent reply other threads:[~2018-04-10 13:59 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 [this message]
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
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=20180410145926.64c4f671@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).