* SIGPIPE (Re: ZSH history not saved anymore) [not found] ` <871tqxqyil.fsf@thinkpad-t440p.tsdh.org> @ 2014-09-27 17:53 ` Bart Schaefer 2014-09-27 20:40 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Bart Schaefer @ 2014-09-27 17:53 UTC (permalink / raw) To: zsh-workers On Sep 27, 10:05am, Tassilo Horn wrote: } Subject: Re: ZSH history not saved anymore } } > Obviously you can work around the problem by telling zsh to trap that } > signal: } > } > TRAPPIPE() { } > exit } > } } } Yes, that works. So I'll keep that in my ~/.zshrc for the time being. I'm a bit hesitant to change this after all these years, but perhaps an interactive shell should exit on SIGPIPE if the terminal is not still open? I'm probably missing something having to do with subshells receiving the PIPE signal. There are regrettably many code paths covering all the ways an interactive shell might fork. diff --git a/Src/init.c b/Src/init.c index 40f46a8..6d005dc 100644 --- a/Src/init.c +++ b/Src/init.c @@ -1134,6 +1134,7 @@ init_signals(void) winch_block(); /* See utils.c:preprompt() */ #endif if (interact) { + install_handler(SIGPIPE); install_handler(SIGALRM); signal_ignore(SIGTERM); } diff --git a/Src/signals.c b/Src/signals.c index cb2b581..094b4b0 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -594,6 +594,17 @@ zhandler(int sig) wait_for_processes(); break; + case SIGPIPE: + if (!handletrap(SIGPIPE)) { + if (!interact) + _exit(SIGPIPE); + else if (!isatty(SHTTY)) { + stopmsg = 1; + zexit(SIGPIPE, 1); + } + } + break; + case SIGHUP: if (!handletrap(SIGHUP)) { stopmsg = 1; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SIGPIPE (Re: ZSH history not saved anymore) 2014-09-27 17:53 ` SIGPIPE (Re: ZSH history not saved anymore) Bart Schaefer @ 2014-09-27 20:40 ` Peter Stephenson 2014-09-27 23:55 ` Bart Schaefer 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2014-09-27 20:40 UTC (permalink / raw) To: zsh-workers On Sat, 27 Sep 2014 10:53:01 -0700 Bart Schaefer <schaefer@brasslantern.com> wrote: > I'm a bit hesitant to change this after all these years, but perhaps an > interactive shell should exit on SIGPIPE if the terminal is not still open? It's hard to see how can that be wrong if we exit on EOF on the terminal. > I'm probably missing something having to do with subshells receiving the > PIPE signal. There are regrettably many code paths covering all the ways > an interactive shell might fork. I don't know what it is that stops it running zexit() and having the same effect in a subshell, hence writing out history incorrectly, if that's what you mean, but you may be thinking of something more subtle. pws ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SIGPIPE (Re: ZSH history not saved anymore) 2014-09-27 20:40 ` Peter Stephenson @ 2014-09-27 23:55 ` Bart Schaefer 2014-09-28 18:04 ` Bart Schaefer 0 siblings, 1 reply; 8+ messages in thread From: Bart Schaefer @ 2014-09-27 23:55 UTC (permalink / raw) To: zsh-workers On Sep 27, 9:40pm, Peter Stephenson wrote: } Subject: Re: SIGPIPE (Re: ZSH history not saved anymore) } } On Sat, 27 Sep 2014 10:53:01 -0700 } Bart Schaefer <schaefer@brasslantern.com> wrote: } > I'm a bit hesitant to change this after all these years, but perhaps an } > interactive shell should exit on SIGPIPE if the terminal is not still open? } } It's hard to see how can that be wrong if we exit on EOF on the terminal. Ideally we'd know what descriptor caused the SIGPIPE and only exit if it was the terminal. isatty(SHTTY) is a crude approximation. } > I'm probably missing something having to do with subshells receiving the } > PIPE signal. } } I don't know what it is that stops it running zexit() and having the } same effect in a subshell, hence writing out history incorrectly, if } that's what you mean, but you may be thinking of something more subtle. Usually a SIGPIPE is generated by a write on a descriptor whose "other end" is closed. So I'm wondering if there are cases where a subshell might get a SIGPIPE on write, in which not only should it not zexit() but it shouldn't exit at all? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SIGPIPE (Re: ZSH history not saved anymore) 2014-09-27 23:55 ` Bart Schaefer @ 2014-09-28 18:04 ` Bart Schaefer 2014-09-28 18:18 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Bart Schaefer @ 2014-09-28 18:04 UTC (permalink / raw) To: zsh-workers On Sep 27, 4:55pm, Bart Schaefer wrote: } } Usually a SIGPIPE is generated by a write on a descriptor whose "other } end" is closed. So I'm wondering if there are cases where a subshell } might get a SIGPIPE on write, in which not only should it not zexit() } but it shouldn't exit at all? Behavior before patch: torch% (sleep 5; echo hello; print -u2 continued after PIPE) | (exit) torch% Behavior after patch in 33257: torch% (sleep 5; echo hello; print -u2 continued after PIPE) | (exit) echo: write error: broken pipe zsh: write error: broken pipe continued after PIPE torch% No history saved in either case, so at least that part is OK, but the patch obviously breaks something in subshells. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SIGPIPE (Re: ZSH history not saved anymore) 2014-09-28 18:04 ` Bart Schaefer @ 2014-09-28 18:18 ` Peter Stephenson 2014-09-28 20:16 ` Bart Schaefer 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2014-09-28 18:18 UTC (permalink / raw) To: zsh-workers On Sun, 28 Sep 2014 11:04:38 -0700 Bart Schaefer <schaefer@brasslantern.com> wrote: > Behavior before patch: > > torch% (sleep 5; echo hello; print -u2 continued after PIPE) | (exit) > torch% > > Behavior after patch in 33257: > > torch% (sleep 5; echo hello; print -u2 continued after PIPE) | (exit) > echo: write error: broken pipe > zsh: write error: broken pipe > continued after PIPE > torch% > > No history saved in either case, so at least that part is OK, but the > patch obviously breaks something in subshells. So in the first case I presume we're exiting (silently) on SIGPIPE. That should be just a question of checking if SIGPIPE is trapped and if it isn't setting the handler to the default in entersubsh(). There's some partial prior art for this. pws ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SIGPIPE (Re: ZSH history not saved anymore) 2014-09-28 18:18 ` Peter Stephenson @ 2014-09-28 20:16 ` Bart Schaefer 2014-09-29 8:45 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Bart Schaefer @ 2014-09-28 20:16 UTC (permalink / raw) To: zsh-workers On Sep 28, 7:18pm, Peter Stephenson wrote: } } So in the first case I presume we're exiting (silently) on SIGPIPE. That } should be just a question of checking if SIGPIPE is trapped and if it isn't } setting the handler to the default in entersubsh(). There's some partial } prior art for this. It gets a little stranger - this is again before the patch: torch% TRAPPIPE() { : } torch% (sleep 5; echo hello; print -u2 $sysparams[pid] continued after PIPE) | (exit) TRAPPIPE: write error: broken pipe echo: write error: broken pipe zsh: write error: broken pipe 3579 continued after PIPE Why did TRAPPIPE() receive a SIGPIPE? And then: torch% TRAPPIPE() { print -u2 $sysparams[pid] } torch% (sleep 5; echo hello; print -u2 $sysparams[pid] continued after PIPE) | (exit) 3659 TRAPPIPE: write error: broken pipe 3659 echo: write error: broken pipe zsh: write error: broken pipe 3659 continued after PIPE Note all calls to TRAPPIPE were in the subshell, and that it was called more than once. I think the following patch now covers keeping all this weird behavior the same, while still zexit()'ng if SHTTY is lost. There may remain cases where a top-level non-interactive shell now calls _exit(SIGPIPE) where it did not before, but I'm not sure how to test for those. Just doing 'kill -PIPE' of 'zsh -fc "sleep 10"' exits 141 both before and after the patch below. TRAPEXIT() has never been called on SIGPIPE. I'm not entirely happy about examining forklevel in the very last hunk, but it's the only value passed out of entersubsh() that seems to make sense for this. That hunk is restoring zsh's handler when a user-defined trap is removed; without it, defining and then removing TRAPPIPE would instead restore the OS default SIGPIPE handling. I'd be grateful if somebody else could invent a regression test for this, perhaps in Test/C03traps.ztst ? diff --git a/Src/exec.c b/Src/exec.c index fb2739a..fbd3095 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -1005,6 +1005,8 @@ entersubsh(int flags) signal_default(SIGTERM); if (!(sigtrapped[SIGINT] & ZSIG_IGNORED)) signal_default(SIGINT); + if (!(sigtrapped[SIGPIPE])) + signal_default(SIGPIPE); } if (!(sigtrapped[SIGQUIT] & ZSIG_IGNORED)) signal_default(SIGQUIT); diff --git a/Src/init.c b/Src/init.c index 40f46a8..6d005dc 100644 --- a/Src/init.c +++ b/Src/init.c @@ -1134,6 +1134,7 @@ init_signals(void) winch_block(); /* See utils.c:preprompt() */ #endif if (interact) { + install_handler(SIGPIPE); install_handler(SIGALRM); signal_ignore(SIGTERM); } diff --git a/Src/signals.c b/Src/signals.c index cb2b581..84a2609 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -594,6 +594,17 @@ zhandler(int sig) wait_for_processes(); break; + case SIGPIPE: + if (!handletrap(SIGPIPE)) { + if (!interact) + _exit(SIGPIPE); + else if (!isatty(SHTTY)) { + stopmsg = 1; + zexit(SIGPIPE, 1); + } + } + break; + case SIGHUP: if (!handletrap(SIGHUP)) { stopmsg = 1; @@ -897,6 +908,8 @@ removetrap(int sig) noholdintr(); } else if (sig == SIGHUP) install_handler(sig); + else if (sig == SIGPIPE && interact && !forklevel) + install_handler(sig); else if (sig && sig <= SIGCOUNT && #ifdef SIGWINCH sig != SIGWINCH && ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SIGPIPE (Re: ZSH history not saved anymore) 2014-09-28 20:16 ` Bart Schaefer @ 2014-09-29 8:45 ` Peter Stephenson 2014-09-29 15:29 ` Bart Schaefer 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2014-09-29 8:45 UTC (permalink / raw) To: zsh-workers On Sun, 28 Sep 2014 13:16:27 -0700 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Sep 28, 7:18pm, Peter Stephenson wrote: > } > } So in the first case I presume we're exiting (silently) on SIGPIPE. That > } should be just a question of checking if SIGPIPE is trapped and if it isn't > } setting the handler to the default in entersubsh(). There's some partial > } prior art for this. > > It gets a little stranger - this is again before the patch: > > torch% TRAPPIPE() { : } > torch% (sleep 5; echo hello; print -u2 $sysparams[pid] continued after PIPE) | > (exit) > TRAPPIPE: write error: broken pipe > echo: write error: broken pipe > zsh: write error: broken pipe > 3579 continued after PIPE > > > Why did TRAPPIPE() receive a SIGPIPE? And then: I think we're looking at this in execcmd() or similar chunks. /* It's a builtin */ if (forked) closem(FDT_INTERNAL); lastval = execbuiltin(args, (Builtin) hn); fflush(stdout); if (save[1] == -2) { if (ferror(stdout)) { zwarn("write error: %e", errno); clearerr(stdout); } } else clearerr(stdout); so it doesn't necessarily mean there's a signal, just that there's a broken pipe. Maybe the fflush() is enough to trigger setting errno again if it failed to write the first time? pws ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SIGPIPE (Re: ZSH history not saved anymore) 2014-09-29 8:45 ` Peter Stephenson @ 2014-09-29 15:29 ` Bart Schaefer 0 siblings, 0 replies; 8+ messages in thread From: Bart Schaefer @ 2014-09-29 15:29 UTC (permalink / raw) To: zsh-workers On Sep 29, 9:45am, Peter Stephenson wrote: } } > Why did TRAPPIPE() receive a SIGPIPE? } } I think we're looking at this in execcmd() or similar chunks. } } /* It's a builtin */ } if (forked) } closem(FDT_INTERNAL); } lastval = execbuiltin(args, (Builtin) hn); } fflush(stdout); } if (save[1] == -2) { } if (ferror(stdout)) { } zwarn("write error: %e", errno); } clearerr(stdout); } } } } else } clearerr(stdout); } } so it doesn't necessarily mean there's a signal, just that there's a } broken pipe. The zwarn() explains why the output appears more than once, but there definitely are two calls to TRAPPIPE, so I think that does mean that there was a signal 2 of the 3 times? } Maybe the fflush() is enough to trigger setting errno } again if it failed to write the first time? The fflush is probably enough to trigger a second SIGPIPE if the shell didn't exit on the first one. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-09-29 15:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <87mw9qdp7s.fsf@thinkpad-t440p.tsdh.org> [not found] ` <20140924200710.2f764272@pws-pc.ntlworld.com> [not found] ` <8738bg2n1v.fsf@thinkpad-t440p.tsdh.org> [not found] ` <140926000448.ZM30835@torch.brasslantern.com> [not found] ` <878ul6lrw9.fsf@thinkpad-t440p.tsdh.org> [not found] ` <CABx2=D9xdeJ0qDNayUG0astemFEtK13SLpA3j8UQT5EqHW_PmA@mail.gmail.com> [not found] ` <87y4t66td0.fsf@thinkpad-t440p.tsdh.org> [not found] ` <CABx2=D-chwqBDZLTk8OqeUDqxvnYUQFFKWbiw7h3ZgUGtSb_CQ@mail.gmail.com> [not found] ` <871tqxqyil.fsf@thinkpad-t440p.tsdh.org> 2014-09-27 17:53 ` SIGPIPE (Re: ZSH history not saved anymore) Bart Schaefer 2014-09-27 20:40 ` Peter Stephenson 2014-09-27 23:55 ` Bart Schaefer 2014-09-28 18:04 ` Bart Schaefer 2014-09-28 18:18 ` Peter Stephenson 2014-09-28 20:16 ` Bart Schaefer 2014-09-29 8:45 ` Peter Stephenson 2014-09-29 15:29 ` 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).