* Signal handling bugaboo in command substitution
@ 2016-03-08 2:44 Bart Schaefer
2016-03-08 10:00 ` Peter Stephenson
0 siblings, 1 reply; 6+ messages in thread
From: Bart Schaefer @ 2016-03-08 2:44 UTC (permalink / raw)
To: zsh-workers
At a PS1 prompt:
torch% print $(sleep 3; echo foo)
Press ctrl+z during the sleep. Zsh is now hung, because the command
substitution is occuring in prefork() so there's nothing to handle the
stopped children and the parent itself ignores the signal. Zsh is hung;
it won't return to a prompt, the command substitution will never produce
the awaited output, and nothing (except "kill -CONT" from another shell)
will wake it back up.
I'm not sure what to do here. In other circumstances it's OK to stop a
command substitution with a ctrl+z, and in any kind of non-interactive
shell or even in a subshell the parent would handle the signal.
Bash appears to leave TSTP blocked here when interactive. I don't think
testing for interactivity is sufficient in zsh context, though.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Signal handling bugaboo in command substitution
2016-03-08 2:44 Signal handling bugaboo in command substitution Bart Schaefer
@ 2016-03-08 10:00 ` Peter Stephenson
2016-03-08 10:12 ` Peter Stephenson
0 siblings, 1 reply; 6+ messages in thread
From: Peter Stephenson @ 2016-03-08 10:00 UTC (permalink / raw)
To: zsh-workers
On Mon, 07 Mar 2016 18:44:06 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> At a PS1 prompt:
>
> torch% print $(sleep 3; echo foo)
>
> Press ctrl+z during the sleep. Zsh is now hung, because the command
> substitution is occuring in prefork() so there's nothing to handle the
> stopped children and the parent itself ignores the signal. Zsh is hung;
> it won't return to a prompt, the command substitution will never produce
> the awaited output, and nothing (except "kill -CONT" from another shell)
> will wake it back up.
>
> I'm not sure what to do here. In other circumstances it's OK to stop a
> command substitution with a ctrl+z, and in any kind of non-interactive
> shell or even in a subshell the parent would handle the signal.
>
> Bash appears to leave TSTP blocked here when interactive. I don't think
> testing for interactivity is sufficient in zsh context, though.
How about something like this?
pws
diff --git a/Src/exec.c b/Src/exec.c
index b60fc90..caeb461 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1001,9 +1001,21 @@ entersubsh(int flags)
* signals. If it is, we need to keep the special behaviour:
* see note about attachtty() above.
*/
- signal_default(SIGTTOU);
- signal_default(SIGTTIN);
- signal_default(SIGTSTP);
+ if (flags & ESUB_NOMONITOR)
+ {
+ /*
+ * Allowing any form of interactive signalling here is
+ * actively harmful as we are in a context where there is no
+ * control over the process.
+ */
+ signal_ignore(SIGTTOU);
+ signal_ignore(SIGTTIN);
+ signal_ignore(SIGTSTP);
+ } else {
+ signal_default(SIGTTOU);
+ signal_default(SIGTTIN);
+ signal_default(SIGTSTP);
+ }
}
if (interact) {
signal_default(SIGTERM);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Signal handling bugaboo in command substitution
2016-03-08 10:00 ` Peter Stephenson
@ 2016-03-08 10:12 ` Peter Stephenson
2016-03-08 17:30 ` Bart Schaefer
0 siblings, 1 reply; 6+ messages in thread
From: Peter Stephenson @ 2016-03-08 10:12 UTC (permalink / raw)
To: zsh-workers
On Tue, 8 Mar 2016 10:00:04 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> How about something like this?
Actually, probably ESUB_NOMONITOR should take precedence. That's really
not a good place to allow this stuff, no matter who thought it might
be...
pws
diff --git a/ChangeLog b/ChangeLog
index 420eb26..ce05a3d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,4 @@
-2016-03-07 Peter Stephenson <p.stephenson@samsung.com>
+-2016-03-07 Peter Stephenson <p.stephenson@samsung.com>
* 38111: Src/parse.c: remove redundant return values from
par_list() and par_list1().
diff --git a/Src/exec.c b/Src/exec.c
index b60fc90..9fec16e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -994,9 +994,19 @@ entersubsh(int flags)
if ((flags & ESUB_REVERTPGRP) && getpid() == mypgrp)
release_pgrp();
shout = NULL;
- if (!job_control_ok) {
+ if (flags & ESUB_NOMONITOR)
+ {
+ /*
+ * Allowing any form of interactive signalling here is
+ * actively harmful as we are in a context where there is no
+ * control over the process.
+ */
+ signal_ignore(SIGTTOU);
+ signal_ignore(SIGTTIN);
+ signal_ignore(SIGTSTP);
+ } else if (!job_control_ok) {
/*
- * If this process is not goign to be doing job control,
+ * If this process is not going to be doing job control,
* we don't want to do special things with the corresponding
* signals. If it is, we need to keep the special behaviour:
* see note about attachtty() above.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Signal handling bugaboo in command substitution
2016-03-08 10:12 ` Peter Stephenson
@ 2016-03-08 17:30 ` Bart Schaefer
2016-03-09 9:55 ` Peter Stephenson
0 siblings, 1 reply; 6+ messages in thread
From: Bart Schaefer @ 2016-03-08 17:30 UTC (permalink / raw)
To: Zsh hackers list
On Tue, Mar 8, 2016 at 2:12 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
>
> Actually, probably ESUB_NOMONITOR should take precedence. That's really
> not a good place to allow this stuff, no matter who thought it might be...
Aside from the spurious ChangeLog diff and the inconsistent
left-curly-brace cuddling, this seems to do what was intended.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Signal handling bugaboo in command substitution
2016-03-08 17:30 ` Bart Schaefer
@ 2016-03-09 9:55 ` Peter Stephenson
2016-03-09 15:37 ` Bart Schaefer
0 siblings, 1 reply; 6+ messages in thread
From: Peter Stephenson @ 2016-03-09 9:55 UTC (permalink / raw)
To: Zsh hackers list
On Tue, 08 Mar 2016 09:30:42 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Tue, Mar 8, 2016 at 2:12 AM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
> >
> > Actually, probably ESUB_NOMONITOR should take precedence. That's really
> > not a good place to allow this stuff, no matter who thought it might be...
>
> Aside from the spurious ChangeLog diff and the inconsistent
> left-curly-brace cuddling, this seems to do what was intended.
It occurs to me there's probably still a very thin race associated with
forking and then calling entersubsh() that may be wider than just this
case. We'd need to queue signals before the fork and unqueue them in
both parent and child at the end of entersubsh(). It might well be more
trouble than it's worth to fix.
pws
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Signal handling bugaboo in command substitution
2016-03-09 9:55 ` Peter Stephenson
@ 2016-03-09 15:37 ` Bart Schaefer
0 siblings, 0 replies; 6+ messages in thread
From: Bart Schaefer @ 2016-03-09 15:37 UTC (permalink / raw)
To: Zsh hackers list
On Mar 9, 9:55am, Peter Stephenson wrote:
}
} It occurs to me there's probably still a very thin race associated with
} forking and then calling entersubsh() that may be wider than just this
} case. We'd need to queue signals before the fork and unqueue them in
} both parent and child at the end of entersubsh(). It might well be more
} trouble than it's worth to fix.
It'd also be wrong -- we don't want both the parent and the child to
handle any queued signal; in the child we'd have to erase the queue,
not just turn off queuing.
There are probably cases where this already could happen if the timing
were right.
Unless somebody comes up with a reproducible (even irregularly so) example,
I think you're right that it's more trouble than worth.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-09 15:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-08 2:44 Signal handling bugaboo in command substitution Bart Schaefer
2016-03-08 10:00 ` Peter Stephenson
2016-03-08 10:12 ` Peter Stephenson
2016-03-08 17:30 ` Bart Schaefer
2016-03-09 9:55 ` Peter Stephenson
2016-03-09 15:37 ` Bart Schaefer
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).