* Zsh: [7] + 23074 suspended (tty output) @ 2018-09-15 17:22 TS 2018-09-15 18:10 ` Bart Schaefer 2018-09-17 14:01 ` Sebastian Gniazdowski 0 siblings, 2 replies; 32+ messages in thread From: TS @ 2018-09-15 17:22 UTC (permalink / raw) To: zsh-workers Hello, i am not subscribed, please cc me. in this bugreport https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818 i reported an issue with Zsh 5.6. Debian maintainers asked to report directly. So here it goes. The shortest test case i could come with is documented in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 Basically its a setup for startup profiling which did work up to 5.5.1 but since 5.6 instead of displaying profiling data zsh says: zsh: suspended (tty output) i then have to 'fg' first to see the actual profiling data. kind regards, Thilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-15 17:22 Zsh: [7] + 23074 suspended (tty output) TS @ 2018-09-15 18:10 ` Bart Schaefer 2018-09-15 18:51 ` TS 2018-09-17 14:01 ` Sebastian Gniazdowski 1 sibling, 1 reply; 32+ messages in thread From: Bart Schaefer @ 2018-09-15 18:10 UTC (permalink / raw) To: TS; +Cc: Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 738 bytes --] 5.6.2 has been announced, with a fix for this. On Sat, Sep 15, 2018, 10:42 AM TS <debts@xk2c.de> wrote: > Hello, > > i am not subscribed, please cc me. > > in this bugreport > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818 > > i reported an issue with Zsh 5.6. Debian maintainers asked to report > directly. So here it goes. > > The shortest test case i could come with is documented in > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 > > Basically its a setup for startup profiling which did work up to 5.5.1 > but since 5.6 instead of displaying profiling data zsh says: > > zsh: suspended (tty output) > > i then have to 'fg' first to see the actual profiling data. > > > > > kind regards, > > Thilo > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-15 18:10 ` Bart Schaefer @ 2018-09-15 18:51 ` TS 2018-09-15 19:06 ` TS 0 siblings, 1 reply; 32+ messages in thread From: TS @ 2018-09-15 18:51 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh hackers list Hello, Bart Schaefer schrieb/wrote: > 5.6.2 has been announced, with a fix for this. Thanks for your reply. Sorry for not being verbose enough. Well actually i did this with 5.6.2 and that version still shows of that issue. Downgrading to 5.5.1 and older fixes it for me. kind regards, Thilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-15 18:51 ` TS @ 2018-09-15 19:06 ` TS 2018-09-16 8:40 ` Vincent Lefevre 0 siblings, 1 reply; 32+ messages in thread From: TS @ 2018-09-15 19:06 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh hackers list TS schrieb/wrote: > Hello, > > Bart Schaefer schrieb/wrote: >> 5.6.2 has been announced, with a fix for this. > > Thanks for your reply. Sorry for not being verbose enough. > Well actually i did this with 5.6.2 and that version still > shows of that issue. > Downgrading to 5.5.1 and older fixes it for me. Test case, tested with 5.6.2: https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=908818;filename=.zshrc;msg=66 result for me: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 kind regards, Thilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-15 19:06 ` TS @ 2018-09-16 8:40 ` Vincent Lefevre 2018-09-16 15:40 ` TS 2018-09-18 1:16 ` Vincent Lefevre 0 siblings, 2 replies; 32+ messages in thread From: Vincent Lefevre @ 2018-09-16 8:40 UTC (permalink / raw) To: TS; +Cc: Bart Schaefer, Zsh hackers list On 2018-09-15 21:06:56 +0200, TS wrote: > Test case, tested with 5.6.2: > https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=908818;filename=.zshrc;msg=66 > > result for me: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 I think I have noticed issues with "less" using zsh-5.6.1-10-g24bb465 but I didn't remember what I did exactly and I couldn't reproduce them. I needed to do a fg. -- Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-16 8:40 ` Vincent Lefevre @ 2018-09-16 15:40 ` TS 2018-09-16 17:51 ` Peter Stephenson 2018-09-17 20:13 ` TS 2018-09-18 1:16 ` Vincent Lefevre 1 sibling, 2 replies; 32+ messages in thread From: TS @ 2018-09-16 15:40 UTC (permalink / raw) To: Bart Schaefer, Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 1137 bytes --] Hello Vincent Lefevre schrieb/wrote: > On 2018-09-15 21:06:56 +0200, TS wrote: >> Test case, tested with 5.6.2: >> https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=908818;filename=.zshrc;msg=66 >> >> result for me: >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 > > I think I have noticed issues with "less" using zsh-5.6.1-10-g24bb465 > but I didn't remember what I did exactly and I couldn't reproduce > them. I needed to do a fg. I have now tried different iterations of that minimal .zshrc. It seems only when less is wrapped inside a function i get this "zsh: suspended (tty output)". But when less is wrapped inside a function like this, it is rather reliable replicable. steps to show the issue: 1) ~/.zshrc as attached 2) exec zsh or login e.g.: % exec zsh zsh: suspended (tty output) i have a testuser for such things, therefore i "su -l" to him first. But according to my tests, the "su -l" part is irrelevant for the issue. Only the "less wrapped in function" part seem to matter. I earlier versions of Zsh that did not make a difference. Thanks for taking a look. kind regards, Thilo [-- Attachment #2: .zshrc --] [-- Type: text/plain, Size: 66 bytes --] builtin zmodload zsh/zprof lessx() { =less } zprof |& lessx ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-16 15:40 ` TS @ 2018-09-16 17:51 ` Peter Stephenson 2018-09-17 20:13 ` TS 1 sibling, 0 replies; 32+ messages in thread From: Peter Stephenson @ 2018-09-16 17:51 UTC (permalink / raw) To: Zsh hackers list On Sun, 16 Sep 2018 17:40:48 +0200 TS <debts@xk2c.de> wrote: > I have now tried different iterations of that minimal .zshrc. > It seems only when less is wrapped inside a function i get this > "zsh: suspended (tty output)". Thanks for that --- it does help to have all the reproduction detail inline (or attached) in a post on the list. > But when less is wrapped inside a function like this, it is rather reliable > replicable. All the previous forms of this bug show this when inside a sourced file, so e.g. testfile containing echo | { cat | more; } followed by ". ./testfile" shows this up. I've rebased job_control_debug2 on top of all the latest fixes to track this down. (Just the sort of thing I like doing in my hotel room on holiday.) The issue is that the list_pipe_job mechanism is being applied twice, once for the "." and once for the nested code (a function in your case, a nested pipeline in the above case). Regular readers will know one application is quite enough --- quite whether doing this twice is a good idea is anybody's guess. Anyway, the first job is marked as being in control of the terminal; then the second job gets the terminal. The first job exits and the shell erroneously thinks it was still in control of the terminal and swipes it back from under the feet of the second. We need to keep track of which job's process group last got the terminal. I think the combination of recording this in attachtty() and also in the recently added code that fixes up the records in the main shell from what the new subshell just did should do the trick --- we're still protected by the STAT_NOSTTY check with the additional safety. However, I'm not sure we want to rush this fix out to all and sundry. The trouble is, clearly no one's testing things until they're released (which is why the releases are broken). If this is likely to come unstuck anywhere, I'd suspect use of ^Z and fg which is applying the most additional stress to the foreground terminal machinations, so if you have things you do with ^Z and complex shell constructs please try them out. I've also tightened up in entersubsh() so that if the process is asynchronous it doesn't pass up the status --- that's all the exec.c hunks are doing. diff --git a/Src/exec.c b/Src/exec.c index b9af9ea63..a667b078d 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -1036,7 +1036,7 @@ entersubsh(int flags, struct entersubsh_ret *retp) if (!(flags & ESUB_ASYNC)) attachtty(jobtab[thisjob].gleader); } - if (retp) { + if (retp && !(flags & ESUB_ASYNC)) { retp->gleader = jobtab[list_pipe_job].gleader; retp->list_pipe_job = list_pipe_job; } @@ -1058,12 +1058,13 @@ entersubsh(int flags, struct entersubsh_ret *retp) !jobtab[list_pipe_job].gleader) jobtab[list_pipe_job].gleader = jobtab[thisjob].gleader; setpgrp(0L, jobtab[thisjob].gleader); - if (!(flags & ESUB_ASYNC)) + if (!(flags & ESUB_ASYNC)) { attachtty(jobtab[thisjob].gleader); - if (retp) { - retp->gleader = jobtab[thisjob].gleader; - if (list_pipe_job != thisjob) - retp->list_pipe_job = list_pipe_job; + if (retp) { + retp->gleader = jobtab[thisjob].gleader; + if (list_pipe_job != thisjob) + retp->list_pipe_job = list_pipe_job; + } } } } diff --git a/Src/jobs.c b/Src/jobs.c index db2e87ec1..2d58319a8 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -40,6 +40,11 @@ mod_export pid_t origpgrp; /**/ mod_export pid_t mypgrp; + +/* the last process group to attach to the terminal */ + +/**/ +pid_t last_attached_pgrp; /* the job we are working on */ @@ -1405,6 +1410,11 @@ addproc(pid_t pid, char *text, int aux, struct timeval *bgtime, jobtab[thisjob].gleader = gleader; if (list_pipe_job_used != -1) jobtab[list_pipe_job_used].gleader = gleader; + /* + * Record here this is the latest process group to grab the + * terminal as attachtty() was run in the subshell. + */ + last_attached_pgrp = gleader; } else if (!jobtab[thisjob].gleader) jobtab[thisjob].gleader = pid; /* attach this process to end of process list of current job */ diff --git a/Src/signals.c b/Src/signals.c index 99aad0fab..26d88abc2 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -540,8 +540,8 @@ wait_for_processes(void) if (WIFEXITED(status) && pn->pid == jn->gleader && killpg(pn->pid, 0) == -1) { - jn->gleader = 0; - if (!(jn->stat & STAT_NOSTTY)) { + if (last_attached_pgrp == jn->gleader && + !(jn->stat & STAT_NOSTTY)) { /* * This PID was in control of the terminal; * reclaim terminal now it has exited. @@ -552,6 +552,7 @@ wait_for_processes(void) attachtty(mypgrp); adjustwinsize(0); } + jn->gleader = 0; } } update_job(jn); diff --git a/Src/utils.c b/Src/utils.c index 075d27241..5a9fbdd32 100644 --- a/Src/utils.c +++ b/Src/utils.c @@ -4670,6 +4670,10 @@ attachtty(pid_t pgrp) ep = 1; } } + else + { + last_attached_pgrp = pgrp; + } } } ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-16 15:40 ` TS 2018-09-16 17:51 ` Peter Stephenson @ 2018-09-17 20:13 ` TS 2018-09-18 17:59 ` Peter Stephenson 1 sibling, 1 reply; 32+ messages in thread From: TS @ 2018-09-17 20:13 UTC (permalink / raw) To: Bart Schaefer, Zsh hackers list, p.w.stephenson Hello Peter, > Thanks for that --- it does help to have all the reproduction detail inline > (or attached) in a post on the list. I take a note about this for next time. Thanks back to you also, for having a look at it. > (Just the sort of thing I like doing in my hotel room on holiday.) Send me your bank connection via private mail. I will transfer you a amount of money for having {1..3} drinks of your liking. ;) Uups, almost forgot the patch you send did fix it. Thanks! kind regards, Thilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-17 20:13 ` TS @ 2018-09-18 17:59 ` Peter Stephenson 0 siblings, 0 replies; 32+ messages in thread From: Peter Stephenson @ 2018-09-18 17:59 UTC (permalink / raw) To: Zsh hackers list On Mon, 17 Sep 2018 22:13:40 +0200 TS <debts@xk2c.de> wrote: > Send me your bank connection via private mail. I will transfer you a amount of > money for having {1..3} drinks of your liking. ;) That's OK, but thank for the thought. > Uups, almost forgot the patch you send did fix it. Thanks! Good --- there's one thing about it I don't understand yet but it shouldn't affect the fix. pws ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-16 8:40 ` Vincent Lefevre 2018-09-16 15:40 ` TS @ 2018-09-18 1:16 ` Vincent Lefevre 2018-09-18 17:58 ` Peter Stephenson 2018-09-18 19:30 ` TS 1 sibling, 2 replies; 32+ messages in thread From: Vincent Lefevre @ 2018-09-18 1:16 UTC (permalink / raw) To: TS, Bart Schaefer, Zsh hackers list On 2018-09-16 10:40:20 +0200, Vincent Lefevre wrote: > On 2018-09-15 21:06:56 +0200, TS wrote: > > Test case, tested with 5.6.2: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=908818;filename=.zshrc;msg=66 > > > > result for me: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 > > I think I have noticed issues with "less" using zsh-5.6.1-10-g24bb465 > but I didn't remember what I did exactly and I couldn't reproduce > them. I needed to do a fg. The session in the terminal looks like that: zira:~/software/mpfr> emacs <3:13:29 ^Zzsh: exit 148 zsh: suspended eclient zira:~/software/mpfr[TSTP]> bg <3:13:33 [1] + continued eclient zira:~/software/mpfr> less src/Makefile <3:13:38 zsh: suspended (tty output) less src/Makefile zira:~/software/mpfr[TTOU]> fg <3:13:42 [2] + continued less src/Makefile zira:~/software/mpfr> <3:13:42 In short: 1. Run emacs[*] (with its X interface). 2. Type Ctrl-Z in the terminal to stop Emacs and get the prompt. 3. Type "bg". 4. View a file with "less"[*]. 5. Quit Emacs (while "less" is still running). 6. Quit "less" (with 'q'). This yields a TTOU on "less" (see above). Note that 2+3 cannot be replaced by the use of "emacs &". [*] This is actually a shell function: eclient () { emulate -LR zsh local display i if [[ -n $GNUCLIENT_ENABLED ]] then display="$DISPLAY" for i in "$@" do [[ "$i" == "-nw" ]] && unset display done [[ -n $display ]] && gnuclient "$@" 2> /dev/null || \emacs "$@" else \emacs "$@" fi } with GNUCLIENT_ENABLED not defined. -- Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-18 1:16 ` Vincent Lefevre @ 2018-09-18 17:58 ` Peter Stephenson 2018-09-20 12:01 ` Vincent Lefevre 2018-09-18 19:30 ` TS 1 sibling, 1 reply; 32+ messages in thread From: Peter Stephenson @ 2018-09-18 17:58 UTC (permalink / raw) To: Zsh hackers list; +Cc: Bart Schaefer On Tue, 18 Sep 2018 03:16:45 +0200 Vincent Lefevre <vincent@vinc17.net> wrote: > 1. Run emacs[*] (with its X interface). > 2. Type Ctrl-Z in the terminal to stop Emacs and get the prompt. > 3. Type "bg". > 4. View a file with "less"[*]. > 5. Quit Emacs (while "less" is still running). > 6. Quit "less" (with 'q'). > > This yields a TTOU on "less" (see above). Note that 2+3 cannot be > replaced by the use of "emacs &". This appears to be the same bug as the last one I've recently fixed. When emacs exits, having previously been in control of the terminal (hence the need to start in the foreground), the shell thinks it still is and swipes the terminal from less. The shell now remembers the actual last process group to be in control. pws ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-18 17:58 ` Peter Stephenson @ 2018-09-20 12:01 ` Vincent Lefevre 2018-09-20 16:11 ` Daniel Shahaf 0 siblings, 1 reply; 32+ messages in thread From: Vincent Lefevre @ 2018-09-20 12:01 UTC (permalink / raw) To: zsh-workers On 2018-09-18 18:58:21 +0100, Peter Stephenson wrote: > On Tue, 18 Sep 2018 03:16:45 +0200 > Vincent Lefevre <vincent@vinc17.net> wrote: > > 1. Run emacs[*] (with its X interface). > > 2. Type Ctrl-Z in the terminal to stop Emacs and get the prompt. > > 3. Type "bg". > > 4. View a file with "less"[*]. > > 5. Quit Emacs (while "less" is still running). > > 6. Quit "less" (with 'q'). > > > > This yields a TTOU on "less" (see above). Note that 2+3 cannot be > > replaced by the use of "emacs &". > > This appears to be the same bug as the last one I've recently fixed. [...] This bug no longer occurs with Debian's zsh 5.6.2-3 package. This is probably this fix: [ Daniel Shahaf ] * [3d8f4eee] Cherry-pick 551ff842 from upstream: Another attachtty() fix. (Closes: #908818) -- Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-20 12:01 ` Vincent Lefevre @ 2018-09-20 16:11 ` Daniel Shahaf 0 siblings, 0 replies; 32+ messages in thread From: Daniel Shahaf @ 2018-09-20 16:11 UTC (permalink / raw) To: zsh-workers Vincent Lefevre wrote on Thu, Sep 20, 2018 at 14:01:06 +0200: > This bug no longer occurs with Debian's zsh 5.6.2-3 package. > This is probably this fix: > > [ Daniel Shahaf ] > * [3d8f4eee] Cherry-pick 551ff842 from upstream: Another attachtty() > fix. (Closes: #908818) For clarity, this is an excerpt from the changelog of the Debian package which names me as the person who backported Peter's patch, which had been committed after the 5.6.2 tag, into the Debian package of 5.6.2. This is simply the default format for Debian package changelogs. (I have my disagreements with it.) Cheers, Daniel ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-18 1:16 ` Vincent Lefevre 2018-09-18 17:58 ` Peter Stephenson @ 2018-09-18 19:30 ` TS 2018-09-19 3:38 ` TS 1 sibling, 1 reply; 32+ messages in thread From: TS @ 2018-09-18 19:30 UTC (permalink / raw) To: Bart Schaefer, Zsh hackers list, vincent Hello Vincent, please refine your bugreport. I am not able to reproduce your report. basically: emacs != eclient % su -l heinb Password: tosh% emacs zsh: suspended emacs tosh% bg [1] + continued emacs tosh% [1] + suspended (tty output) emacs tosh% i now undestand upsteam. this is with zsh git tag 5.6.2 kind regards, Thilo Vincent Lefevre schrieb/wrote: > On 2018-09-16 10:40:20 +0200, Vincent Lefevre wrote: >> On 2018-09-15 21:06:56 +0200, TS wrote: >>> Test case, tested with 5.6.2: >>> https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=908818;filename=.zshrc;msg=66 >>> >>> result for me: >>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 >> >> I think I have noticed issues with "less" using zsh-5.6.1-10-g24bb465 >> but I didn't remember what I did exactly and I couldn't reproduce >> them. I needed to do a fg. > > The session in the terminal looks like that: > > zira:~/software/mpfr> emacs <3:13:29 > ^Zzsh: exit 148 > zsh: suspended eclient > zira:~/software/mpfr[TSTP]> bg <3:13:33 > [1] + continued eclient > zira:~/software/mpfr> less src/Makefile <3:13:38 > zsh: suspended (tty output) less src/Makefile > zira:~/software/mpfr[TTOU]> fg <3:13:42 > [2] + continued less src/Makefile > zira:~/software/mpfr> <3:13:42 > > In short: > > 1. Run emacs[*] (with its X interface). > 2. Type Ctrl-Z in the terminal to stop Emacs and get the prompt. > 3. Type "bg". > 4. View a file with "less"[*]. > 5. Quit Emacs (while "less" is still running). > 6. Quit "less" (with 'q'). > > This yields a TTOU on "less" (see above). Note that 2+3 cannot be > replaced by the use of "emacs &". > > [*] This is actually a shell function: > > eclient () { > emulate -LR zsh > local display i > if [[ -n $GNUCLIENT_ENABLED ]] > then > display="$DISPLAY" > for i in "$@" > do > [[ "$i" == "-nw" ]] && unset display > done > [[ -n $display ]] && gnuclient "$@" 2> /dev/null || \emacs "$@" > else > \emacs "$@" > fi > } > > with GNUCLIENT_ENABLED not defined. > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-18 19:30 ` TS @ 2018-09-19 3:38 ` TS 2018-09-19 14:23 ` Daniel Tameling 2018-09-19 16:12 ` Peter Stephenson 0 siblings, 2 replies; 32+ messages in thread From: TS @ 2018-09-19 3:38 UTC (permalink / raw) To: Bart Schaefer, Zsh hackers list, vincent, p.w.stephenson [-- Attachment #1: Type: text/plain, Size: 3282 bytes --] Hello Peter, when emacs is wrapped inside function, then emacs is dead when: ^Z bg Does not happen when emacs called w/o function. See minimal .zshrc attached. % su -l heinb Password: tosh% emacs zsh: suspended emacs tosh% bg [1] + continued emacs tosh% [1] + suspended (tty output) emacs tosh% jobs -l [1] + 20708 suspended (tty output) emacs tosh% fg %1 [1] + continued emacs tosh% eclient zsh: suspended eclient tosh% jobs -l [1] + 20718 suspended (signal) eclient tosh% bg [1] + continued eclient tosh% jobs -l [1] - 20718 running eclient tosh% fg %1 [1] - running eclient <<<#### here terminal is broken This is still even after 551ff842721d6ca83727dbe6cd40178f46cc8201 aka "43464: Another attachtty() fix." HTH kind regards, Thilo TS schrieb/wrote: > Hello Vincent, > > please refine your bugreport. I am not able to reproduce your report. > basically: emacs != eclient > > % su -l heinb > Password: > tosh% emacs > > zsh: suspended emacs > tosh% bg > [1] + continued emacs > tosh% > [1] + suspended (tty output) emacs > tosh% > > i now undestand upsteam. > > this is with zsh git tag 5.6.2 > > > > kind regards, > > Thilo > > Vincent Lefevre schrieb/wrote: >> On 2018-09-16 10:40:20 +0200, Vincent Lefevre wrote: >>> On 2018-09-15 21:06:56 +0200, TS wrote: >>>> Test case, tested with 5.6.2: >>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=908818;filename=.zshrc;msg=66 >>>> >>>> result for me: >>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 >>> >>> I think I have noticed issues with "less" using zsh-5.6.1-10-g24bb465 >>> but I didn't remember what I did exactly and I couldn't reproduce >>> them. I needed to do a fg. >> >> The session in the terminal looks like that: >> >> zira:~/software/mpfr> emacs <3:13:29 >> ^Zzsh: exit 148 >> zsh: suspended eclient >> zira:~/software/mpfr[TSTP]> bg <3:13:33 >> [1] + continued eclient >> zira:~/software/mpfr> less src/Makefile <3:13:38 >> zsh: suspended (tty output) less src/Makefile >> zira:~/software/mpfr[TTOU]> fg <3:13:42 >> [2] + continued less src/Makefile >> zira:~/software/mpfr> <3:13:42 >> >> In short: >> >> 1. Run emacs[*] (with its X interface). >> 2. Type Ctrl-Z in the terminal to stop Emacs and get the prompt. >> 3. Type "bg". >> 4. View a file with "less"[*]. >> 5. Quit Emacs (while "less" is still running). >> 6. Quit "less" (with 'q'). >> >> This yields a TTOU on "less" (see above). Note that 2+3 cannot be >> replaced by the use of "emacs &". >> >> [*] This is actually a shell function: >> >> eclient () { >> emulate -LR zsh >> local display i >> if [[ -n $GNUCLIENT_ENABLED ]] >> then >> display="$DISPLAY" >> for i in "$@" >> do >> [[ "$i" == "-nw" ]] && unset display >> done >> [[ -n $display ]] && gnuclient "$@" 2> /dev/null || \emacs "$@" >> else >> \emacs "$@" >> fi >> } >> >> with GNUCLIENT_ENABLED not defined. >> > [-- Attachment #2: .zshrc --] [-- Type: text/plain, Size: 51 bytes --] setopt NO_globalrcs eclient () { =emacs "$@" } ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-19 3:38 ` TS @ 2018-09-19 14:23 ` Daniel Tameling 2018-09-19 16:12 ` Peter Stephenson 1 sibling, 0 replies; 32+ messages in thread From: Daniel Tameling @ 2018-09-19 14:23 UTC (permalink / raw) To: zsh-workers es On Wed, Sep 19, 2018 at 05:38:11AM +0200, TS wrote: > Hello Peter, > > when emacs is wrapped inside function, > then emacs is dead when: > > ^Z > bg > > Does not happen when emacs called w/o function. > See minimal .zshrc attached. > > > % su -l heinb > Password: > tosh% emacs > > zsh: suspended emacs > tosh% bg > [1] + continued emacs > tosh% > [1] + suspended (tty output) emacs > tosh% jobs -l > [1] + 20708 suspended (tty output) emacs > tosh% fg %1 > [1] + continued emacs > tosh% eclient > zsh: suspended eclient > tosh% jobs -l > [1] + 20718 suspended (signal) eclient > tosh% bg > [1] + continued eclient > tosh% jobs -l > [1] - 20718 running eclient > tosh% fg %1 > [1] - running eclient > <<<#### here terminal is broken > > > setopt NO_globalrcs > eclient () { > =emacs "$@" > } > > This is not a new phenomenon. I can reproduce it with zsh 5.3. ps shows one difference between the function and the normal emacs case: there is another zsh instance: function case after issuing bg: $ ps -o pid,ppid,state,command 80367 80366 Ss /bin/zsh 80900 80367 T+ /opt/local/bin/emacs 80902 80367 T /bin/zsh normal case after issuing bg: $ ps -o pid,ppid,state,command 81266 81265 Ss /bin/zsh 81297 81266 T /opt/local/bin/emacs After fg %1, I can revive the function case by sending SIGCONT directly to the emacs process. -- Best, Daniel ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-19 3:38 ` TS 2018-09-19 14:23 ` Daniel Tameling @ 2018-09-19 16:12 ` Peter Stephenson 2018-09-19 18:15 ` TS 1 sibling, 1 reply; 32+ messages in thread From: Peter Stephenson @ 2018-09-19 16:12 UTC (permalink / raw) To: TS; +Cc: Bart Schaefer, Zsh hackers list, vincent On Wed, 19 Sep 2018 05:38:11 +0200 TS <debts@xk2c.de> wrote: > when emacs is wrapped inside function, > then emacs is dead when: > > ^Z > bg > > Does not happen when emacs called w/o function. > See minimal .zshrc attached. > > % su -l heinb > Password: > tosh% emacs > > zsh: suspended emacs > tosh% bg > [1] + continued emacs > tosh% > [1] + suspended (tty output) emacs I'm not quite sure what you're doing here, but it seems to involve emacs being suspended due to terminal output when it goes into the *background*, which is entirely sensible as it's not even supposed to be attached to the terminal then. Quite when it attempts to write to the terminal during this period is not up to the shell. I'm not sure what effect the function is having but I don't see how it can be affecting writes from emacs. My best guess would be it's adding some kind of delay which is affecting how Emacs starts, but the actual interactions going on are obscure. I'm not actually seeing this behaviour myself anyway, but it sounds like a race so that's probably not surprising. This doesn't appear to be the problem Vincent was reporting, which I can't get to happen with that minimal eclient(). pws ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-19 16:12 ` Peter Stephenson @ 2018-09-19 18:15 ` TS 0 siblings, 0 replies; 32+ messages in thread From: TS @ 2018-09-19 18:15 UTC (permalink / raw) To: Peter Stephenson; +Cc: Bart Schaefer, Zsh hackers list, vincent Hello, Peter Stephenson schrieb/wrote: >> when emacs is wrapped inside function, >> then emacs is dead when: >> >> ^Z >> bg >> >> Does not happen when emacs called w/o function. >> See minimal .zshrc attached. >> >> % su -l heinb >> Password: >> tosh% emacs >> >> zsh: suspended emacs >> tosh% bg >> [1] + continued emacs >> tosh% >> [1] + suspended (tty output) emacs > > I'm not quite sure what you're doing here, Ok will write down as detailed as possible: First i have an user with absolute no configuration apart from the .zshrc given before. That is to make sure i test in as clean as possible environment. For that i now moved the "setopt NO_globalrcs" from .zshrc to .zshenv to make even cleaner. commented lines are, well comments: ## I then login as that user: % su -l heinb Password: ## I start emacs and press ^z tosh% emacs zsh: suspended emacs ## enter bg at prompt tosh% bg [1] + continued emacs tosh% [1] + suspended (tty output) emacs ## verify backgrounded jobs tosh% jobs -l [1] + 3179 suspended (tty output) emacs ## foreground emacs again and normal quit aka ^x^c tosh% fg %1 [1] + continued emacs tosh% ## now the same sequence with function: tosh% which eclient eclient () { =emacs "$@" } ## I start eclient and press ^z tosh% eclient zsh: suspended eclient ## enter bg at prompt tosh% bg [1] + continued eclient ## verify backgrounded jobs tosh% jobs -l [1] - 3289 running eclient ## foreground eclient again tosh% fg %1 [1] - running eclient ^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C After that emacs does not come back and terminal is not useable anymore, prompt does not appear and ^c does not help to return. If can help by testing or other informations just let me know. In case of trace or so i will need detailed instructions what to do but will happily help. > This doesn't appear to be the problem Vincent was reporting, which I > can't get to happen with that minimal eclient(). In this case sorry Vicent for the complaint. Actually by trying to figuring out an easy way to reproduce your issue we potential found an other one. I am confidant that is half way to fix that, too. Which is good IMHO. kind regards, Thilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Zsh: [7] + 23074 suspended (tty output) 2018-09-15 17:22 Zsh: [7] + 23074 suspended (tty output) TS 2018-09-15 18:10 ` Bart Schaefer @ 2018-09-17 14:01 ` Sebastian Gniazdowski 2018-09-17 14:49 ` Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) Daniel Shahaf 1 sibling, 1 reply; 32+ messages in thread From: Sebastian Gniazdowski @ 2018-09-17 14:01 UTC (permalink / raw) To: Zsh hackers list Hello Allow me to do this broken record writing, but wouldn't it be nice to have Valgrind automatic tests prepared for a handy run while doing the `suspended tty' investigation and patching? The VATS (Valgrind Automatic Tests Suite) can be still added to upstream. It is just a copy of Test directory, I know this doesn't sound nice, but also: definition-files with non-harmful, already known Valgrind errors, and a Zsh script that parses Valgrind output doing some hiding, formatting, etc. On Sat, 15 Sep 2018 at 19:42, TS <debts@xk2c.de> wrote: > > Hello, > > i am not subscribed, please cc me. > > in this bugreport > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818 > > i reported an issue with Zsh 5.6. Debian maintainers asked to report > directly. So here it goes. > > The shortest test case i could come with is documented in > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908818#66 > > Basically its a setup for startup profiling which did work up to 5.5.1 > but since 5.6 instead of displaying profiling data zsh says: > > zsh: suspended (tty output) > > i then have to 'fg' first to see the actual profiling data. > > > > > kind regards, > > Thilo > -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 32+ messages in thread
* Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2018-09-17 14:01 ` Sebastian Gniazdowski @ 2018-09-17 14:49 ` Daniel Shahaf 2018-09-18 5:21 ` Sebastian Gniazdowski 0 siblings, 1 reply; 32+ messages in thread From: Daniel Shahaf @ 2018-09-17 14:49 UTC (permalink / raw) To: zsh-workers Sebastian Gniazdowski wrote on Mon, 17 Sep 2018 16:01 +0200: > Hello > Allow me to do this broken record writing, but wouldn't it be nice to > have Valgrind automatic tests prepared for a handy run while doing the > `suspended tty' investigation and patching? The VATS (Valgrind > Automatic Tests Suite) can be still added to upstream. It is just a > copy of Test directory, I know this doesn't sound nice, but also: > definition-files with non-harmful, already known Valgrind errors, and > a Zsh script that parses Valgrind output doing some hiding, > formatting, etc. Yes, it would be a good thing to have support in the build system to run the test suite under valgrind, complete with developer documentation (such as what configure and config.modules settings to use). However, if the proposal means having two copies of the Test/ directory, then it's a non-starter. Can you describe what changes would be involved? Would the test suite remain as portable as it should be? Cheers, Daniel ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2018-09-17 14:49 ` Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) Daniel Shahaf @ 2018-09-18 5:21 ` Sebastian Gniazdowski 2018-09-18 15:55 ` Daniel Shahaf 0 siblings, 1 reply; 32+ messages in thread From: Sebastian Gniazdowski @ 2018-09-18 5:21 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list On Mon, 17 Sep 2018 at 16:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Can you describe what changes would be involved? Would the test > suite remain as portable as it should be? I was thinking little different back then when creating VATS, it appeared reasonable to have separate tests aimed at stressing Zsh in terms of memory usage, etc. I now see that the main Zsh tests are putting much stress on Zsh, so VATS should just add vtest.conf, custom runtests.zsh, zsh-valgrind-parse.cmd, custom ztst.zsh (I'm not sure if the two custom files really have to be different), from here: https://github.com/zdharma/VATS-zsh/tree/master/VATS. The readme of this project is unnecessarily very long, the thing is very simple, I need some time to approach this again with original Test/ in mind. I've found asciinema about VATS: https://asciinema.org/a/125035 (asciinema seems down at the moment). -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2018-09-18 5:21 ` Sebastian Gniazdowski @ 2018-09-18 15:55 ` Daniel Shahaf 2018-09-19 5:04 ` Sebastian Gniazdowski 2019-07-03 23:29 ` Sebastian Gniazdowski 0 siblings, 2 replies; 32+ messages in thread From: Daniel Shahaf @ 2018-09-18 15:55 UTC (permalink / raw) To: Sebastian Gniazdowski; +Cc: Zsh hackers list Sebastian Gniazdowski wrote on Tue, 18 Sep 2018 07:21 +0200: > On Mon, 17 Sep 2018 at 16:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > Can you describe what changes would be involved? Would the test > > suite remain as portable as it should be? > > I was thinking little different back then when creating VATS, it > appeared reasonable to have separate tests aimed at stressing Zsh in > terms of memory usage, etc. I now see that the main Zsh tests are > putting much stress on Zsh, so VATS should just add vtest.conf, custom > runtests.zsh, zsh-valgrind-parse.cmd, custom ztst.zsh (I'm not sure if > the two custom files really have to be different), from here: > https://github.com/zdharma/VATS-zsh/tree/master/VATS. You haven't actually answered my question. Can you, please, explain what changes would need to be made to zsh in order to implement your upthread proposal? It's fine to link to external sources for details but the gist should be self-contained and understandable by readers familiar with zsh and its build system but nothing else. > The readme of this project is unnecessarily very long, the thing is > very simple, I need some time to approach this again with original > Test/ in mind. For avoidance of doubt, I am simply asking for information on the proposal. I am not yet sold on it. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2018-09-18 15:55 ` Daniel Shahaf @ 2018-09-19 5:04 ` Sebastian Gniazdowski 2019-07-03 23:29 ` Sebastian Gniazdowski 1 sibling, 0 replies; 32+ messages in thread From: Sebastian Gniazdowski @ 2018-09-19 5:04 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 472 bytes --] Wt., 18.09.2018, 17:55 użytkownik Daniel Shahaf <d.s@daniel.shahaf.name> napisał: Sebastian Gniazdowski wrote on Tue, 18 Sep 2018 07:21 > so VATS should just add vtest.conf, custom > runtests.zsh, zsh-valgrind-parse.cmd, custom ztst.zsh (I'm not sure if > the two custom files really have to be different), from here: > https://github.com/zdharma/VATS-zsh/tree/master/VATS. I'm answering from phone. The changes are described above, max 4 files addition. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2018-09-18 15:55 ` Daniel Shahaf 2018-09-19 5:04 ` Sebastian Gniazdowski @ 2019-07-03 23:29 ` Sebastian Gniazdowski 2019-07-03 23:31 ` Sebastian Gniazdowski 2019-07-15 9:29 ` Peter Stephenson 1 sibling, 2 replies; 32+ messages in thread From: Sebastian Gniazdowski @ 2019-07-03 23:29 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list On Tue, 18 Sep 2018 at 17:55, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Sebastian Gniazdowski wrote on Tue, 18 Sep 2018 07:21 +0200: > > On Mon, 17 Sep 2018 at 16:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > > Can you describe what changes would be involved? Would the test > > > suite remain as portable as it should be? What's needed is currently only this +-2 patch. It makes passing VALGRIND=1 to the tests' make run the tests through VATS/valgrind. It creates a conditional variable VLGRND which has a value of "v" in such case, making the tests go throguh vruntests.zsh and not runtests.zsh: @@ -35,6 +35,7 @@ VPATH = @srcdir@ sdir = @srcdir@ sdir_top = @top_srcdir@ INSTALL = @INSTALL@ +VLGRND = $(VALGRIND:1=v) @DEFS_MK@ @@ -49,7 +50,7 @@ check test: do echo $$f; done`" \ ZTST_srcdir="$(sdir)" \ ZTST_exe=$(dir_top)/Src/zsh@EXEEXT@ \ - $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/runtests.zsh; then \ + $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/$(VLGRND)runtests.zsh; then \ stat=0; \ else \ stat=1; \ > > The readme of this project is unnecessarily very long, the thing is > > very simple, I need some time to approach this again with original > > Test/ in mind. > > For avoidance of doubt, I am simply asking for information on the > proposal. I am not yet sold on it. I hope the valgrind tests will be added. I've spend 2 days on carefully building the errors database on OS X. I think that on Linux there will be no such problem, as it seems that it's OS X that embraces the fact that freed memory is actually never returned to the OS (so it apparently does allocations here and there, without any code to release the memory). The tests apparently reveal one memory leak currently: ==52847== 79 (24 direct, 55 indirect) bytes in 1 blocks are definitely lost in loss record 349 of 550 ==52847== at 0x10017B545: malloc (vg_replace_malloc.c:302) ==52847== by 0x10004899E: zalloc (mem.c:966) ==52847== by 0x10004316D: znewlinklist (linklist.c:120) ==52847== by 0x10003989D: addfilelist (jobs.c:1297) ==52847== by 0x100017AC0: getoutputfile (exec.c:4796) ==52847== by 0x100078474: stringsubst (subst.c:254) ==52847== by 0x100077E46: prefork (subst.c:142) ==52847== by 0x10001A24F: execfuncdef (exec.c:2567) ==52847== by 0x10001DC23: execcmd_exec (exec.c:3896) ==52847== by 0x10001AAF6: execpline2 (exec.c:1927) ==52847== by 0x100015938: execpline (exec.c:1658) ==52847== by 0x1000150E2: execlist (exec.c:1413) The full procedure to add the valgrind tests to Zsh is: 1. Apply the patch 2. Add files: vruntests.zsh, vtest.conf, zsh-valgrind-parse.cmd to the Test subdirectory After this running make VALGRIND=1 inside the Test subdirectory will run the valgrind tests. The files and additional information are available at: https://github.com/zdharma/VATS-zsh -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-03 23:29 ` Sebastian Gniazdowski @ 2019-07-03 23:31 ` Sebastian Gniazdowski 2019-07-03 23:45 ` Sebastian Gniazdowski 2019-07-15 9:29 ` Peter Stephenson 1 sibling, 1 reply; 32+ messages in thread From: Sebastian Gniazdowski @ 2019-07-03 23:31 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list Ah, also: using the ztst.zsh from the repo will cause the descriptions of all tests to be printed before executing – to allow matching up the valgrind reports against concrete Zsh code. The change to the file is very simple: diff --git a/Test/ztst.zsh b/Test/ztst.zsh index d835e0edb..d20893f53 100755 --- a/Test/ztst.zsh +++ b/Test/ztst.zsh @@ -402,6 +402,7 @@ $ZTST_curline" ZTST_xstatus=$match[1] ZTST_flags=$match[2] ZTST_message=${match[3]:+${match[3][2,-1]}} + [[ $VLGRND_TEST_DESC = 1 ]] && print -- "VATS-test-desc: $ZTST_message" else ZTST_testfailed "expecting test status at: On Thu, 4 Jul 2019 at 01:29, Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote: > > On Tue, 18 Sep 2018 at 17:55, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > Sebastian Gniazdowski wrote on Tue, 18 Sep 2018 07:21 +0200: > > > On Mon, 17 Sep 2018 at 16:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > > > > Can you describe what changes would be involved? Would the test > > > > suite remain as portable as it should be? > > What's needed is currently only this +-2 patch. It makes passing > VALGRIND=1 to the tests' make run the tests through VATS/valgrind. It > creates a conditional variable VLGRND which has a value of "v" in such > case, making the tests go throguh vruntests.zsh and not runtests.zsh: > > @@ -35,6 +35,7 @@ VPATH = @srcdir@ > sdir = @srcdir@ > sdir_top = @top_srcdir@ > INSTALL = @INSTALL@ > +VLGRND = $(VALGRIND:1=v) > > @DEFS_MK@ > > @@ -49,7 +50,7 @@ check test: > do echo $$f; done`" \ > ZTST_srcdir="$(sdir)" \ > ZTST_exe=$(dir_top)/Src/zsh@EXEEXT@ \ > - $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/runtests.zsh; then \ > + $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/$(VLGRND)runtests.zsh; then \ > stat=0; \ > else \ > stat=1; \ > > > > The readme of this project is unnecessarily very long, the thing is > > > very simple, I need some time to approach this again with original > > > Test/ in mind. > > > > For avoidance of doubt, I am simply asking for information on the > > proposal. I am not yet sold on it. > > I hope the valgrind tests will be added. I've spend 2 days on > carefully building the errors database on OS X. I think that on Linux > there will be no such problem, as it seems that it's OS X that > embraces the fact that freed memory is actually never returned to the > OS (so it apparently does allocations here and there, without any code > to release the memory). The tests apparently reveal one memory leak > currently: > > ==52847== 79 (24 direct, 55 indirect) bytes in 1 blocks are definitely > lost in loss record 349 of 550 > ==52847== at 0x10017B545: malloc (vg_replace_malloc.c:302) > ==52847== by 0x10004899E: zalloc (mem.c:966) > ==52847== by 0x10004316D: znewlinklist (linklist.c:120) > ==52847== by 0x10003989D: addfilelist (jobs.c:1297) > ==52847== by 0x100017AC0: getoutputfile (exec.c:4796) > ==52847== by 0x100078474: stringsubst (subst.c:254) > ==52847== by 0x100077E46: prefork (subst.c:142) > ==52847== by 0x10001A24F: execfuncdef (exec.c:2567) > ==52847== by 0x10001DC23: execcmd_exec (exec.c:3896) > ==52847== by 0x10001AAF6: execpline2 (exec.c:1927) > ==52847== by 0x100015938: execpline (exec.c:1658) > ==52847== by 0x1000150E2: execlist (exec.c:1413) > > The full procedure to add the valgrind tests to Zsh is: > 1. Apply the patch > 2. Add files: vruntests.zsh, vtest.conf, zsh-valgrind-parse.cmd to the > Test subdirectory > > After this running make VALGRIND=1 inside the Test subdirectory will > run the valgrind tests. > > The files and additional information are available at: > https://github.com/zdharma/VATS-zsh > > -- > Sebastian Gniazdowski > News: https://twitter.com/ZdharmaI > IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin > Blog: http://zdharma.org -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-03 23:31 ` Sebastian Gniazdowski @ 2019-07-03 23:45 ` Sebastian Gniazdowski 2019-07-13 14:31 ` Sebastian Gniazdowski 0 siblings, 1 reply; 32+ messages in thread From: Sebastian Gniazdowski @ 2019-07-03 23:45 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list I've made a 20 seconds asciinema to show the new test-descriptions feature: https://asciinema.org/a/255371 On Thu, 4 Jul 2019 at 01:31, Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote: > > Ah, also: using the ztst.zsh from the repo will cause the descriptions > of all tests to be printed before executing – to allow matching up the > valgrind reports against concrete Zsh code. The change to the file is > very simple: > > diff --git a/Test/ztst.zsh b/Test/ztst.zsh > index d835e0edb..d20893f53 100755 > --- a/Test/ztst.zsh > +++ b/Test/ztst.zsh > @@ -402,6 +402,7 @@ $ZTST_curline" > ZTST_xstatus=$match[1] > ZTST_flags=$match[2] > ZTST_message=${match[3]:+${match[3][2,-1]}} > + [[ $VLGRND_TEST_DESC = 1 ]] && print -- "VATS-test-desc: > $ZTST_message" > else > ZTST_testfailed "expecting test status at: > > On Thu, 4 Jul 2019 at 01:29, Sebastian Gniazdowski > <sgniazdowski@gmail.com> wrote: > > > > On Tue, 18 Sep 2018 at 17:55, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > > > Sebastian Gniazdowski wrote on Tue, 18 Sep 2018 07:21 +0200: > > > > On Mon, 17 Sep 2018 at 16:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > > > > > > Can you describe what changes would be involved? Would the test > > > > > suite remain as portable as it should be? > > > > What's needed is currently only this +-2 patch. It makes passing > > VALGRIND=1 to the tests' make run the tests through VATS/valgrind. It > > creates a conditional variable VLGRND which has a value of "v" in such > > case, making the tests go throguh vruntests.zsh and not runtests.zsh: > > > > @@ -35,6 +35,7 @@ VPATH = @srcdir@ > > sdir = @srcdir@ > > sdir_top = @top_srcdir@ > > INSTALL = @INSTALL@ > > +VLGRND = $(VALGRIND:1=v) > > > > @DEFS_MK@ > > > > @@ -49,7 +50,7 @@ check test: > > do echo $$f; done`" \ > > ZTST_srcdir="$(sdir)" \ > > ZTST_exe=$(dir_top)/Src/zsh@EXEEXT@ \ > > - $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/runtests.zsh; then \ > > + $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/$(VLGRND)runtests.zsh; then \ > > stat=0; \ > > else \ > > stat=1; \ > > > > > > The readme of this project is unnecessarily very long, the thing is > > > > very simple, I need some time to approach this again with original > > > > Test/ in mind. > > > > > > For avoidance of doubt, I am simply asking for information on the > > > proposal. I am not yet sold on it. > > > > I hope the valgrind tests will be added. I've spend 2 days on > > carefully building the errors database on OS X. I think that on Linux > > there will be no such problem, as it seems that it's OS X that > > embraces the fact that freed memory is actually never returned to the > > OS (so it apparently does allocations here and there, without any code > > to release the memory). The tests apparently reveal one memory leak > > currently: > > > > ==52847== 79 (24 direct, 55 indirect) bytes in 1 blocks are definitely > > lost in loss record 349 of 550 > > ==52847== at 0x10017B545: malloc (vg_replace_malloc.c:302) > > ==52847== by 0x10004899E: zalloc (mem.c:966) > > ==52847== by 0x10004316D: znewlinklist (linklist.c:120) > > ==52847== by 0x10003989D: addfilelist (jobs.c:1297) > > ==52847== by 0x100017AC0: getoutputfile (exec.c:4796) > > ==52847== by 0x100078474: stringsubst (subst.c:254) > > ==52847== by 0x100077E46: prefork (subst.c:142) > > ==52847== by 0x10001A24F: execfuncdef (exec.c:2567) > > ==52847== by 0x10001DC23: execcmd_exec (exec.c:3896) > > ==52847== by 0x10001AAF6: execpline2 (exec.c:1927) > > ==52847== by 0x100015938: execpline (exec.c:1658) > > ==52847== by 0x1000150E2: execlist (exec.c:1413) > > > > The full procedure to add the valgrind tests to Zsh is: > > 1. Apply the patch > > 2. Add files: vruntests.zsh, vtest.conf, zsh-valgrind-parse.cmd to the > > Test subdirectory > > > > After this running make VALGRIND=1 inside the Test subdirectory will > > run the valgrind tests. > > > > The files and additional information are available at: > > https://github.com/zdharma/VATS-zsh > > > > -- > > Sebastian Gniazdowski > > News: https://twitter.com/ZdharmaI > > IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin > > Blog: http://zdharma.org > > > > -- > Sebastian Gniazdowski > News: https://twitter.com/ZdharmaI > IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin > Blog: http://zdharma.org -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-03 23:45 ` Sebastian Gniazdowski @ 2019-07-13 14:31 ` Sebastian Gniazdowski 2019-07-14 6:39 ` Daniel Shahaf 0 siblings, 1 reply; 32+ messages in thread From: Sebastian Gniazdowski @ 2019-07-13 14:31 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list Hello, Daniel seems to be currently away or uninterested – could someone else take a look at this? On Thu, 4 Jul 2019 at 01:45, Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote: > > I've made a 20 seconds asciinema to show the new test-descriptions > feature: https://asciinema.org/a/255371 > > On Thu, 4 Jul 2019 at 01:31, Sebastian Gniazdowski > <sgniazdowski@gmail.com> wrote: > > > > Ah, also: using the ztst.zsh from the repo will cause the descriptions > > of all tests to be printed before executing – to allow matching up the > > valgrind reports against concrete Zsh code. The change to the file is > > very simple: > > > > diff --git a/Test/ztst.zsh b/Test/ztst.zsh > > index d835e0edb..d20893f53 100755 > > --- a/Test/ztst.zsh > > +++ b/Test/ztst.zsh > > @@ -402,6 +402,7 @@ $ZTST_curline" > > ZTST_xstatus=$match[1] > > ZTST_flags=$match[2] > > ZTST_message=${match[3]:+${match[3][2,-1]}} > > + [[ $VLGRND_TEST_DESC = 1 ]] && print -- "VATS-test-desc: > > $ZTST_message" > > else > > ZTST_testfailed "expecting test status at: > > > > On Thu, 4 Jul 2019 at 01:29, Sebastian Gniazdowski > > <sgniazdowski@gmail.com> wrote: > > > > > > On Tue, 18 Sep 2018 at 17:55, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > > > > > Sebastian Gniazdowski wrote on Tue, 18 Sep 2018 07:21 +0200: > > > > > On Mon, 17 Sep 2018 at 16:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > > > > > > > > Can you describe what changes would be involved? Would the test > > > > > > suite remain as portable as it should be? > > > > > > What's needed is currently only this +-2 patch. It makes passing > > > VALGRIND=1 to the tests' make run the tests through VATS/valgrind. It > > > creates a conditional variable VLGRND which has a value of "v" in such > > > case, making the tests go throguh vruntests.zsh and not runtests.zsh: > > > > > > @@ -35,6 +35,7 @@ VPATH = @srcdir@ > > > sdir = @srcdir@ > > > sdir_top = @top_srcdir@ > > > INSTALL = @INSTALL@ > > > +VLGRND = $(VALGRIND:1=v) > > > > > > @DEFS_MK@ > > > > > > @@ -49,7 +50,7 @@ check test: > > > do echo $$f; done`" \ > > > ZTST_srcdir="$(sdir)" \ > > > ZTST_exe=$(dir_top)/Src/zsh@EXEEXT@ \ > > > - $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/runtests.zsh; then \ > > > + $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/$(VLGRND)runtests.zsh; then \ > > > stat=0; \ > > > else \ > > > stat=1; \ > > > > > > > > The readme of this project is unnecessarily very long, the thing is > > > > > very simple, I need some time to approach this again with original > > > > > Test/ in mind. > > > > > > > > For avoidance of doubt, I am simply asking for information on the > > > > proposal. I am not yet sold on it. > > > > > > I hope the valgrind tests will be added. I've spend 2 days on > > > carefully building the errors database on OS X. I think that on Linux > > > there will be no such problem, as it seems that it's OS X that > > > embraces the fact that freed memory is actually never returned to the > > > OS (so it apparently does allocations here and there, without any code > > > to release the memory). The tests apparently reveal one memory leak > > > currently: > > > > > > ==52847== 79 (24 direct, 55 indirect) bytes in 1 blocks are definitely > > > lost in loss record 349 of 550 > > > ==52847== at 0x10017B545: malloc (vg_replace_malloc.c:302) > > > ==52847== by 0x10004899E: zalloc (mem.c:966) > > > ==52847== by 0x10004316D: znewlinklist (linklist.c:120) > > > ==52847== by 0x10003989D: addfilelist (jobs.c:1297) > > > ==52847== by 0x100017AC0: getoutputfile (exec.c:4796) > > > ==52847== by 0x100078474: stringsubst (subst.c:254) > > > ==52847== by 0x100077E46: prefork (subst.c:142) > > > ==52847== by 0x10001A24F: execfuncdef (exec.c:2567) > > > ==52847== by 0x10001DC23: execcmd_exec (exec.c:3896) > > > ==52847== by 0x10001AAF6: execpline2 (exec.c:1927) > > > ==52847== by 0x100015938: execpline (exec.c:1658) > > > ==52847== by 0x1000150E2: execlist (exec.c:1413) > > > > > > The full procedure to add the valgrind tests to Zsh is: > > > 1. Apply the patch > > > 2. Add files: vruntests.zsh, vtest.conf, zsh-valgrind-parse.cmd to the > > > Test subdirectory > > > > > > After this running make VALGRIND=1 inside the Test subdirectory will > > > run the valgrind tests. > > > > > > The files and additional information are available at: > > > https://github.com/zdharma/VATS-zsh > > > > > > -- > > > Sebastian Gniazdowski > > > News: https://twitter.com/ZdharmaI > > > IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin > > > Blog: http://zdharma.org > > > > > > > > -- > > Sebastian Gniazdowski > > News: https://twitter.com/ZdharmaI > > IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin > > Blog: http://zdharma.org > > > > -- > Sebastian Gniazdowski > News: https://twitter.com/ZdharmaI > IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin > Blog: http://zdharma.org -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-13 14:31 ` Sebastian Gniazdowski @ 2019-07-14 6:39 ` Daniel Shahaf 2019-07-14 22:17 ` Sebastian Gniazdowski 0 siblings, 1 reply; 32+ messages in thread From: Daniel Shahaf @ 2019-07-14 6:39 UTC (permalink / raw) To: Sebastian Gniazdowski; +Cc: Zsh hackers list Sebastian Gniazdowski wrote on Sat, Jul 13, 2019 at 16:31:10 +0200: > Daniel seems to be currently away or uninterested – could someone else > take a look at this? You're not entitled to having me review your patch, nor did I at any point commit myself to doing so. Do not insinuate otherwise. As to your actual patch, no, I do not presently have time to review it. Daniel ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-14 6:39 ` Daniel Shahaf @ 2019-07-14 22:17 ` Sebastian Gniazdowski 2019-07-15 8:54 ` Peter Stephenson 0 siblings, 1 reply; 32+ messages in thread From: Sebastian Gniazdowski @ 2019-07-14 22:17 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh hackers list On Sun, 14 Jul 2019 at 08:39, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Sebastian Gniazdowski wrote on Sat, Jul 13, 2019 at 16:31:10 +0200: > > Daniel seems to be currently away or uninterested – could someone else > > take a look at this? > > You're not entitled to having me review your patch, nor did I at any > point commit myself to doing so. Do not insinuate otherwise. I wasn't leveraging any "entitlement" but just sincerely following your shown interest to the Valgrind tests. Sorry if it turned out to sound differently. > As to your actual patch, no, I do not presently have time to review it. Ok. I've waited 9 days and I'm afraid that the patch will get lost somewhere. I just want to say, that the thing isn't actually a patch. It's a tool that makes the current tests go through Valgrind. The patch that adds the 'VALGRIND=1' variable to the tests' Makefile is a +-2 lines patch. Therefore could someone else take a look? There's not much to review. -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-14 22:17 ` Sebastian Gniazdowski @ 2019-07-15 8:54 ` Peter Stephenson 0 siblings, 0 replies; 32+ messages in thread From: Peter Stephenson @ 2019-07-15 8:54 UTC (permalink / raw) To: zsh-workers On Mon, 2019-07-15 at 00:17 +0200, Sebastian Gniazdowski wrote: > On Sun, 14 Jul 2019 at 08:39, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Ok. I've waited 9 days and I'm afraid that the patch will get lost > somewhere. I just want to say, that the thing isn't actually a patch. > It's a tool that makes the current tests go through Valgrind. The > patch that adds the 'VALGRIND=1' variable to the tests' Makefile is a > +-2 lines patch. Therefore could someone else take a look? There's not > much to review. I don't mind the handler code changing, to make it easy to run tests slightly different, but I'd prefer to add a more general mechanism that's more like how the current system works... This would mean running make ZTST_handler=vruntests.zsh or you can simply set that in the environment. pws diff --git a/Test/Makefile.in b/Test/Makefile.in index 083df4942..09f37bf53 100644 --- a/Test/Makefile.in +++ b/Test/Makefile.in @@ -40,16 +40,21 @@ INSTALL = @INSTALL@ # ========== DEPENDENCIES FOR TESTING ========== +# If ZTST_handler is not empty, run that instead of runtests.zsh. +# It's assumed to be found in the source directory. check test: if test -n "$(DLLD)"; then \ cd $(dir_top) && DESTDIR= \ $(MAKE) MODDIR=`pwd`/$(subdir)/Modules install.modules > /dev/null; \ fi + if test -z "$$ZTST_handler"; then \ + ZTST_handler=runtests.zsh; \ + fi; \ if ZTST_testlist="`for f in $(sdir)/$(TESTNUM)*.ztst; \ do echo $$f; done`" \ ZTST_srcdir="$(sdir)" \ ZTST_exe=$(dir_top)/Src/zsh@EXEEXT@ \ - $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/runtests.zsh; then \ + $(dir_top)/Src/zsh@EXEEXT@ +Z -f $(sdir)/$$ZTST_handler; then \ stat=0; \ else \ stat=1; \ ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-03 23:29 ` Sebastian Gniazdowski 2019-07-03 23:31 ` Sebastian Gniazdowski @ 2019-07-15 9:29 ` Peter Stephenson 2019-07-15 18:45 ` Sebastian Gniazdowski 1 sibling, 1 reply; 32+ messages in thread From: Peter Stephenson @ 2019-07-15 9:29 UTC (permalink / raw) To: zsh-workers On Thu, 2019-07-04 at 01:29 +0200, Sebastian Gniazdowski wrote: > The tests apparently reveal one memory leak currently: > > ==52847== 79 (24 direct, 55 indirect) bytes in 1 blocks are definitely > lost in loss record 349 of 550 > ==52847== at 0x10017B545: malloc (vg_replace_malloc.c:302) > ==52847== by 0x10004899E: zalloc (mem.c:966) > ==52847== by 0x10004316D: znewlinklist (linklist.c:120) > ==52847== by 0x10003989D: addfilelist (jobs.c:1297) > ==52847== by 0x100017AC0: getoutputfile (exec.c:4796) > ==52847== by 0x100078474: stringsubst (subst.c:254) > ==52847== by 0x100077E46: prefork (subst.c:142) > ==52847== by 0x10001A24F: execfuncdef (exec.c:2567) > ==52847== by 0x10001DC23: execcmd_exec (exec.c:3896) > ==52847== by 0x10001AAF6: execpline2 (exec.c:1927) > ==52847== by 0x100015938: execpline (exec.c:1658) > ==52847== by 0x1000150E2: execlist (exec.c:1413) By the way, I think these are benign --- addfilelist() adds a file to be tidied up when a job exits, but these appear to happening in subshells. This means there's no job clear-up --- but the shell simply exits. So the memory isn't recovered in the subshell but that doesn't cause a long-term build-up. I haven't followed every single one of this type, though. pws ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) 2019-07-15 9:29 ` Peter Stephenson @ 2019-07-15 18:45 ` Sebastian Gniazdowski 0 siblings, 0 replies; 32+ messages in thread From: Sebastian Gniazdowski @ 2019-07-15 18:45 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh hackers list On Mon, 15 Jul 2019 at 11:30, Peter Stephenson <p.stephenson@samsung.com> wrote: > but these appear to happening in subshells. > This means there's no job clear-up --- but the shell simply exits. So > the memory isn't recovered in the subshell but that doesn't cause a > long-term build-up. Yes that makes sense. > I haven't followed every single one of this type, though. So have you run the tests? On which OS? I'm wondering if the errors database was good / enough complete. ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2019-07-15 18:45 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-15 17:22 Zsh: [7] + 23074 suspended (tty output) TS 2018-09-15 18:10 ` Bart Schaefer 2018-09-15 18:51 ` TS 2018-09-15 19:06 ` TS 2018-09-16 8:40 ` Vincent Lefevre 2018-09-16 15:40 ` TS 2018-09-16 17:51 ` Peter Stephenson 2018-09-17 20:13 ` TS 2018-09-18 17:59 ` Peter Stephenson 2018-09-18 1:16 ` Vincent Lefevre 2018-09-18 17:58 ` Peter Stephenson 2018-09-20 12:01 ` Vincent Lefevre 2018-09-20 16:11 ` Daniel Shahaf 2018-09-18 19:30 ` TS 2018-09-19 3:38 ` TS 2018-09-19 14:23 ` Daniel Tameling 2018-09-19 16:12 ` Peter Stephenson 2018-09-19 18:15 ` TS 2018-09-17 14:01 ` Sebastian Gniazdowski 2018-09-17 14:49 ` Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) Daniel Shahaf 2018-09-18 5:21 ` Sebastian Gniazdowski 2018-09-18 15:55 ` Daniel Shahaf 2018-09-19 5:04 ` Sebastian Gniazdowski 2019-07-03 23:29 ` Sebastian Gniazdowski 2019-07-03 23:31 ` Sebastian Gniazdowski 2019-07-03 23:45 ` Sebastian Gniazdowski 2019-07-13 14:31 ` Sebastian Gniazdowski 2019-07-14 6:39 ` Daniel Shahaf 2019-07-14 22:17 ` Sebastian Gniazdowski 2019-07-15 8:54 ` Peter Stephenson 2019-07-15 9:29 ` Peter Stephenson 2019-07-15 18:45 ` Sebastian Gniazdowski
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).