* Re: precmd: write error: interrupted [not found] ` <klc0n1$34u$1@ger.gmane.org> @ 2013-04-26 0:53 ` Bart Schaefer 0 siblings, 0 replies; 12+ messages in thread From: Bart Schaefer @ 2013-04-26 0:53 UTC (permalink / raw) To: zsh-workers, Yuri D'Elia, zsh-users (Redirecting this to -workers, so Yuri Cc'd in case he's only on -users) On Apr 25, 9:38pm, Yuri D'Elia wrote: } } precmd() { { print x } 2>/dev/null } } } still doesn't suppress the error. Interesting! Something is restoring the stderr descriptors before the error message in bin_print is written. Here is a bit of strace from 'zsh -f' with { print -n $'x\r' } 2>/devnull running in a loop on the command line (not from a precmd): open("/dev/null", O_WRONLY|O_CREAT|O_NOCTTY|O_TRUNC|O_LARGEFILE, 0666) = 3 dup2(3, 2) = 2 write(1, "x\r", 2) = ? ERESTARTSYS (To be restarted) --- SIGWINCH (Window changed) @ 0 (0) --- dup2(12, 2) = 2 dup2(11, 2) = 2 write(10, "\33[1m\33[7m%\33[27m\33[1m\33[0m \r \r", 105) = 105 write(2, "print: write error: interrupt\n", 30) = 30 Those dup2 calls should happen only at the end of execcmd after bin_print has already returned, but somehow they appear to be happening after the signal handler is called but before the error message. Attaching with a debugger blocks the interrupt so I haven't been able to stack-trace the source of the dup2 calls. ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20130425193817.2f82b60c@pws-pc.ntlworld.com>]
[parent not found: <130425151839.ZM17476@torch.brasslantern.com>]
* Re: precmd: write error: interrupted [not found] ` <130425151839.ZM17476@torch.brasslantern.com> @ 2013-04-26 8:42 ` Peter Stephenson 0 siblings, 0 replies; 12+ messages in thread From: Peter Stephenson @ 2013-04-26 8:42 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 25 Apr 2013 15:18:39 -0700 Bart Schaefer <schaefer@brasslantern.com> wrote: > Consider something like: > > x=({1..1000000} > print $x > > If you can't ^C that print, you're potentially in a world of pain. (It's > already enough of a problem that you can't ^C the assignment itself.) (I moved this bit to zsh-workers.) OK, so what I'm missing is that while the shell code might whizz through there could be long delays in the backend outputting it to the terminal, during which the shell is still executing bin_print() (and that's still true even given your later remarks about queueing). > I'm pretty sure SIGWINCH is an outlier case here and we should focus on > the question of when the shell SHOULD react to window size changes, > rather trying to identify all the builtins that should NOT react. > > For example, we might *always* queue the SIGWINCH signal except when the > shell is blocked in zleread (or is about to, but hasn't yet, displayed > the prompt if ZLE is not active). Those probably don't cover all the > cases, but you get the idea. I can see that it's an infrequent operation that's there to fix things up for the user, i.e. needs handling on the timescale of human reactions rather than, say, the timescale of Unix processes. So yes, it looks plausible it could be handled differently. I suppose if a foreground process is running, it has to be done as at present: the process gets the signal and zsh handles it when it gets back control (but my knowledge of process groups is a bit basic). pws ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <klc2ah$jiv$1@ger.gmane.org>]
[parent not found: <130426080805.ZM18619__18102.73175729$1366989065$gmane$org@torch.brasslantern.com>]
* Re: precmd: write error: interrupted [not found] ` <130426080805.ZM18619__18102.73175729$1366989065$gmane$org@torch.brasslantern.com> @ 2013-04-26 17:59 ` Yuri D'Elia 0 siblings, 0 replies; 12+ messages in thread From: Yuri D'Elia @ 2013-04-26 17:59 UTC (permalink / raw) To: zsh-workers; +Cc: zsh-users On 04/26/2013 05:08 PM, Bart Schaefer wrote: > } What other syscalls would be interrupted by SIGWINCH that shouldn't be > } restarted? Right now I cannot think of anything that SIGWINCH should > } interrupt. > > I've been thinking about this, and the problem with using SA_RESTART is > twofold: > > (1) [Minor] Some platforms don't have restartable syscalls, so this won't > work everywhere. But perhaps the intersection of non-restarable syscalls > and support for SIGWINCH is empty. (I'm somehow curious of which systems don't support SIGWINCH, must be particularly old). > (2) [Potentially major] A user-defined trap can be installed for the > SIGWINCH signal. That means arbitrary shell code might execute during > handling of the signal, so all sorts of things might happen mid-write, > not just the default ioctls. I see, and now I also see your reasoning about queuing SIGWINCH everywhere except when waiting in zleread. At any rate, that's the only point where updating the terminal for the upcoming output makes sense. Updating the column count (for example) while in the middle of a widget expansion for instance won't likely help (or maybe even break some invariant). ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <130426080805.ZM18619@torch.brasslantern.com>]
[parent not found: <517C0E09.4040505@users.sourceforge.net>]
* Re: precmd: write error: interrupted [not found] ` <517C0E09.4040505@users.sourceforge.net> @ 2013-04-27 22:31 ` Bart Schaefer 2013-04-28 15:30 ` Yuri D'Elia 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 2013-04-27 22:31 UTC (permalink / raw) To: Yuri D'Elia; +Cc: zsh-workers [Cc'ing -workers] On Apr 27, 7:42pm, Yuri D'Elia wrote: } Subject: Re: precmd: write error: interrupted } } Just for my information, is there something I should do for these 2 } issues (redirection and SIGWINCH); ie: file a bug report somewhere so it } doesn't get forgotten? The mailing list is the place to file bug reports, so for the moment you have done as much as you can ... } I've seen this error for too long to let it go :) The WINCH bug was tickling a memory so I went looking through the source for mentions of SIGWINCH and found this in zle_main.c:zlread(): /* * On some windowing systems we may enter this function before the * terminal is fully opened and sized, resulting in an infinite * series of SIGWINCH when the handler prints the prompt before we * have done so here. Therefore, hold any such signal until the * first full refresh has completed. The important bit is that the * handler must not see zleactive = 1 until ZLE really is active. * See the end of adjustwinsize() in Src/utils.c */ queue_signals(); What you've been reporting is almost certainly another variation of the same bug. So maybe all we really need is this (plus some #ifdef): diff --git a/Src/utils.c b/Src/utils.c index 26e2a5c..a20a5e1 100644 --- a/Src/utils.c +++ b/Src/utils.c @@ -1323,7 +1323,9 @@ preprompt(void) /* If a shell function named "precmd" exists, * * then execute it. */ + signal_block(signal_mask(SIGWINCH)); /* See zleread() */ callhookfunc("precmd", NULL, 1, NULL); + signal_unblock(signal_mask(SIGWINCH)); if (errflag) return; There are a couple of other things in preprompt() that might want this kind of wrapper too, e.g. periodic and the prepromptfns array, but we can start with checking if the above is helpful. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-04-27 22:31 ` Bart Schaefer @ 2013-04-28 15:30 ` Yuri D'Elia 2013-04-29 1:03 ` Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Yuri D'Elia @ 2013-04-28 15:30 UTC (permalink / raw) To: Bart Schaefer; +Cc: Yuri D'Elia, zsh-workers On 04/28/2013 12:31 AM, Bart Schaefer wrote: > What you've been reporting is almost certainly another variation of the > same bug. So maybe all we really need is this (plus some #ifdef): > > diff --git a/Src/utils.c b/Src/utils.c > index 26e2a5c..a20a5e1 100644 > --- a/Src/utils.c > +++ b/Src/utils.c > @@ -1323,7 +1323,9 @@ preprompt(void) > > /* If a shell function named "precmd" exists, * > * then execute it. */ > + signal_block(signal_mask(SIGWINCH)); /* See zleread() */ > callhookfunc("precmd", NULL, 1, NULL); > + signal_unblock(signal_mask(SIGWINCH)); > if (errflag) > return; This is going to solve the issue for "precmd" only though, not the SIGWHINCH issue as a whole. You can still arbitrarily stop the example loop I shown before by resizing the terminal window. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-04-28 15:30 ` Yuri D'Elia @ 2013-04-29 1:03 ` Bart Schaefer 2013-04-29 1:59 ` Bart Schaefer 2013-05-05 0:01 ` Frank Terbeck 0 siblings, 2 replies; 12+ messages in thread From: Bart Schaefer @ 2013-04-29 1:03 UTC (permalink / raw) To: zsh-workers On Apr 28, 5:30pm, Yuri D'Elia wrote: } Subject: Re: precmd: write error: interrupted } } > + signal_block(signal_mask(SIGWINCH)); /* See zleread() */ } > callhookfunc("precmd", NULL, 1, NULL); } > + signal_unblock(signal_mask(SIGWINCH)); } } This is going to solve the issue for "precmd" only though, not the } SIGWHINCH issue as a whole. True; I was trying to creep up on it incrementally. Incidentally even queue_signals() doesn't actually block the signals, it just doesn't call anything below the top-level handler, so I suspect that replacing the above with queue_signals() would still show the bug. Here's a stab at a more comprehensive patch. This follows the pattern of child_block()/child_unblock() which we use elsewhere to reap children only when safe to do so. I'm not 100% confident that this does the right thing for non-interactive shells/sourcing of scripts, but I *think* the change in shingetline() should cover those cases. diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c index 9a4265f..569ad5f 100644 --- a/Src/Zle/zle_main.c +++ b/Src/Zle/zle_main.c @@ -567,7 +567,9 @@ raw_getbyte(long do_keytmout, char *cptr) gettyinfo(&ti); ti.tio.c_cc[VMIN] = 0; settyinfo(&ti); + winch_unblock(); ret = read(SHTTY, cptr, 1); + winch_block(); ti.tio.c_cc[VMIN] = 1; settyinfo(&ti); if (ret > 0) @@ -597,7 +599,9 @@ raw_getbyte(long do_keytmout, char *cptr) else poll_timeout = -1; + winch_unblock(); selret = poll(fds, errtry ? 1 : nfds, poll_timeout); + winch_block(); # else int fdmax = SHTTY; struct timeval *tvptr; @@ -622,8 +626,10 @@ raw_getbyte(long do_keytmout, char *cptr) else tvptr = NULL; + winch_unblock(); selret = select(fdmax+1, (SELECT_ARG_2_T) & foofd, NULL, NULL, tvptr); + winch_block(); # endif /* * Make sure a user interrupt gets passed on straight away. @@ -788,7 +794,9 @@ raw_getbyte(long do_keytmout, char *cptr) # else ioctl(SHTTY, TCSETA, &ti.tio); # endif + winch_unblock(); ret = read(SHTTY, cptr, 1); + winch_block(); # ifdef HAVE_TERMIOS_H tcsetattr(SHTTY, TCSANOW, &shttyinfo.tio); # else @@ -799,7 +807,9 @@ raw_getbyte(long do_keytmout, char *cptr) #endif } + winch_unblock(); ret = read(SHTTY, cptr, 1); + winch_block(); return ret; } diff --git a/Src/init.c b/Src/init.c index 8467a73..f4fb3be 100644 --- a/Src/init.c +++ b/Src/init.c @@ -1113,6 +1113,7 @@ init_signals(void) install_handler(SIGCHLD); #ifdef SIGWINCH install_handler(SIGWINCH); + winch_block(); /* See utils.c:preprompt() */ #endif if (interact) { install_handler(SIGALRM); diff --git a/Src/input.c b/Src/input.c index 5cff22d..9bd9663 100644 --- a/Src/input.c +++ b/Src/input.c @@ -143,10 +143,12 @@ shingetline(void) p = buf; for (;;) { + winch_unblock(); do { errno = 0; c = fgetc(bshin); } while (c < 0 && errno == EINTR); + winch_block(); if (c < 0 || c == '\n') { if (c == '\n') *p++ = '\n'; diff --git a/Src/signals.h b/Src/signals.h index 9541a1a..d680968 100644 --- a/Src/signals.h +++ b/Src/signals.h @@ -59,6 +59,14 @@ #define child_block() signal_block(sigchld_mask) #define child_unblock() signal_unblock(sigchld_mask) +#ifdef SIGWINCH +# define winch_block() signal_block(signal_mask(SIGWINCH)) +# define winch_unblock() signal_unblock(signal_mask(SIGWINCH)) +#else +# define winch_block() 0 +# define winch_unblock() 0 +#endif + /* ignore a signal */ #define signal_ignore(S) signal(S, SIG_IGN) diff --git a/Src/utils.c b/Src/utils.c index 26e2a5c..94ae522 100644 --- a/Src/utils.c +++ b/Src/utils.c @@ -1291,6 +1291,13 @@ preprompt(void) int period = getiparam("PERIOD"); int mailcheck = getiparam("MAILCHECK"); + /* + * Handle any pending window size changes before we compute prompts, + * then block them again to avoid interrupts during prompt display. + */ + winch_unblock(); + winch_block(); + if (isset(PROMPTSP) && isset(PROMPTCR) && !use_exit_printed && shout) { /* The PROMPT_SP heuristic will move the prompt down to a new line * if there was any dangling output on the line (assuming the terminal -- 1.7.9.6 (Apple Git-31.1) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-04-29 1:03 ` Bart Schaefer @ 2013-04-29 1:59 ` Bart Schaefer 2013-05-05 0:01 ` Frank Terbeck 1 sibling, 0 replies; 12+ messages in thread From: Bart Schaefer @ 2013-04-29 1:59 UTC (permalink / raw) To: zsh-workers On Apr 28, 6:03pm, Bart Schaefer wrote: } } Here's a stab at a more comprehensive patch. This follows the pattern of } child_block()/child_unblock() which we use elsewhere to reap children only } when safe to do so. Incidentally if you're simultaneously doing something like scrolling around with arrow keys with your left hand while resizing the window with your right hand, the shell is probably going to misinterpret some of your keystrokes, even with this patch. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-04-29 1:03 ` Bart Schaefer 2013-04-29 1:59 ` Bart Schaefer @ 2013-05-05 0:01 ` Frank Terbeck 2013-05-05 6:52 ` Bart Schaefer 1 sibling, 1 reply; 12+ messages in thread From: Frank Terbeck @ 2013-05-05 0:01 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers Hi Bart, Bart Schaefer wrote: [...] > Here's a stab at a more comprehensive patch. This follows the pattern of > child_block()/child_unblock() which we use elsewhere to reap children only > when safe to do so. > > I'm not 100% confident that this does the right thing for non-interactive > shells/sourcing of scripts, but I *think* the change in shingetline() > should cover those cases. I'm afraid these changes break SIGWINCH handling of child processes. If you start an editor or mail-reader or terminal-multiplexer (or whatever really), that child will not react to any terminal-resizes anymore. It took a while to find out, why this: TRAPWINCH() { print $COLUMNS $LINES; } didn't trigger in shells inside the terminal-multiplexer I use, but does trigger in a naked xterm without multiplexer. The fact that the terminal multiplexer itself didn't react to SIGWINCHes anymore steered me into zsh's direction (well, not directly but eventually). Building a version of the shell before these changes¹ makes everything work again. Sorry to be the bearer of bad news. ;) Regards, Frank ¹ * cbf6f14 <Jun T> 31357: _cp: add support for Mac OS X (5 days ago) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-05-05 0:01 ` Frank Terbeck @ 2013-05-05 6:52 ` Bart Schaefer 2013-05-05 9:38 ` Frank Terbeck 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 2013-05-05 6:52 UTC (permalink / raw) To: Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 1222 bytes --] On Sat, May 4, 2013 at 5:01 PM, Frank Terbeck <ft@bewatermyfriend.org>wrote: > Hi Bart, > > I'm afraid these changes break SIGWINCH handling of child processes. > > Not entirely surprising. I wondered about that but tried it with a couple of apps before posting and they seemed to be responding to the change in size. I didn't actually test whether they were receiving WINCH, though. The below should help, but someone will have to commit it for me, because the computer with my sourceforge ssh keys suffered a video chip meltdown yesterday and I haven't had a chance to repair it or pull the key file from backup yet. For the same reason, I'm composing this in gmail, so I hope the patch has not been mangled. Something similar may also be needed in the zpty and clone modules. (Has anyone ever used the clone module for anything?) diff --git a/Src/exec.c b/Src/exec.c index fa14875..c71f3f3 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -303,11 +303,13 @@ zfork(struct timeval *tv) zerr("fork failed: %e", errno); return -1; } + if (!pid) { + winch_unblock(); #ifdef HAVE_GETRLIMIT - if (!pid) /* set resource limits for the child process */ setlimits(NULL); #endif + } return pid; } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-05-05 6:52 ` Bart Schaefer @ 2013-05-05 9:38 ` Frank Terbeck 2013-05-05 17:53 ` Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Frank Terbeck @ 2013-05-05 9:38 UTC (permalink / raw) To: Zsh hackers list Bart Schaefer wrote: > On Sat, May 4, 2013 at 5:01 PM, Frank Terbeck <ft@bewatermyfriend.org>wrote: >> I'm afraid these changes break SIGWINCH handling of child processes. > > Not entirely surprising. I wondered about that but tried it with a couple > of apps before posting and they seemed to be responding to the change in > size. I didn't actually test whether they were receiving WINCH, though. > > The below should help, but someone will have to commit it for me, because It does help with normal children (ie. the ones the shell fork()s for). But it doesn't fix cases, in which you use `exec' to replace the shell process with the child process. > the computer with my sourceforge ssh keys suffered a video chip meltdown > yesterday and I haven't had a chance to repair it or pull the key file from > backup yet. For the same reason, I'm composing this in gmail, so I hope > the patch has not been mangled. It was mangled, but is was short enough to apply manually. I've pushed this, because it's definitely an improvement. I tried looking for the right position to unblock SIGWINCH for the `exec' case in exec.c, but couldn't find it yet. Maybe someone has a more specific idea than I do? Regards, Frank ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-05-05 9:38 ` Frank Terbeck @ 2013-05-05 17:53 ` Bart Schaefer 2013-05-05 18:37 ` Frank Terbeck 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 2013-05-05 17:53 UTC (permalink / raw) To: Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 526 bytes --] On Sun, May 5, 2013 at 2:38 AM, Frank Terbeck <ft@bewatermyfriend.org>wrote: > I've pushed this, because it's definitely an improvement. I tried > looking for the right position to unblock SIGWINCH for the `exec' case > in exec.c, but couldn't find it yet. Maybe someone has a more specific > idea than I do? > As far as I can tell, there are six calls to execve() in Src/exec.c. If a winch_unblock() were placed just before each of those six calls, the unblock in zfork() from that last patch could probably be reverted. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: precmd: write error: interrupted 2013-05-05 17:53 ` Bart Schaefer @ 2013-05-05 18:37 ` Frank Terbeck 0 siblings, 0 replies; 12+ messages in thread From: Frank Terbeck @ 2013-05-05 18:37 UTC (permalink / raw) To: Zsh hackers list Bart Schaefer wrote: > On Sun, May 5, 2013 at 2:38 AM, Frank Terbeck <ft@bewatermyfriend.org>wrote: >> I've pushed this, because it's definitely an improvement. I tried >> looking for the right position to unblock SIGWINCH for the `exec' case >> in exec.c, but couldn't find it yet. Maybe someone has a more specific >> idea than I do? > > As far as I can tell, there are six calls to execve() in Src/exec.c. If a > winch_unblock() were placed just before each of those six calls, the > unblock in zfork() from that last patch could probably be reverted. That sounds like a plan. And my test cases seem to work fine, so I just pushed those two changes. Regards, Frank ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-05-05 18:43 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <klbmnc$ieh$1@ger.gmane.org> [not found] ` <130425111646.ZM17258@torch.brasslantern.com> [not found] ` <klc0n1$34u$1@ger.gmane.org> 2013-04-26 0:53 ` precmd: write error: interrupted Bart Schaefer [not found] ` <20130425193817.2f82b60c@pws-pc.ntlworld.com> [not found] ` <130425151839.ZM17476@torch.brasslantern.com> 2013-04-26 8:42 ` Peter Stephenson [not found] ` <klc2ah$jiv$1@ger.gmane.org> [not found] ` <130426080805.ZM18619__18102.73175729$1366989065$gmane$org@torch.brasslantern.com> 2013-04-26 17:59 ` Yuri D'Elia [not found] ` <130426080805.ZM18619@torch.brasslantern.com> [not found] ` <517C0E09.4040505@users.sourceforge.net> 2013-04-27 22:31 ` Bart Schaefer 2013-04-28 15:30 ` Yuri D'Elia 2013-04-29 1:03 ` Bart Schaefer 2013-04-29 1:59 ` Bart Schaefer 2013-05-05 0:01 ` Frank Terbeck 2013-05-05 6:52 ` Bart Schaefer 2013-05-05 9:38 ` Frank Terbeck 2013-05-05 17:53 ` Bart Schaefer 2013-05-05 18:37 ` Frank Terbeck
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).