zsh-users
 help / color / mirror / code / Atom feed
* Zsh spins in endless loop with SIGHUP + read in zshexit
@ 2021-05-13 10:17 Jörg Sommer
  2021-05-13 14:36 ` Daniel Shahaf
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Jörg Sommer @ 2021-05-13 10:17 UTC (permalink / raw)
  To: zsh-users

[-- Attachment #1: Type: text/plain, Size: 7498 bytes --]

Hi,

I'm using `read` in `zshexit` and when my terminal crashes (e.g. XTerm gets
killed or ssh connection dies) Zsh spins in an endless loop until I kill it
with SIGKILL (SIGTERM doesn't work).

```
zshexit()
{
    if [[ ${#${(M)${(f)"$(</proc/self/mountinfo)"}#*/ / rw,}} == 1 ]]
    then
        if read -rq junk"?Root filesystem is still read-write. Remount ro? (y/N) "
        then
            echo
            mount -o remount,ro / || sleep 4
        else
            echo
        fi
    fi
}
```

Here's a backtrace:

```
#0  0x00007fb6d84eda5d in sigprocmask () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00005643fcc03d83 in signal_unblock (set=...) at ../../Src/signals.c:274
#2  0x00007fb6d7fcee1a in raw_getbyte (full=0, cptr=0x7fff401336af "", do_keytmout=0) at ../../../Src/Zle/zle_main.c:844
#3  getbyte (do_keytmout=0, timeout=0x0, full=full@entry=0) at ../../../Src/Zle/zle_main.c:880
#4  0x00007fb6d7fd22ad in zle_main_entry (cmd=<optimized out>, ap=<optimized out>) at ../../../Src/Zle/zle_main.c:2142
#5  0x00005643fcbcbcb8 in zleentry (cmd=7) at ../../Src/init.c:1604
#6  0x00005643fcb9e9c8 in bin_read (name=<optimized out>, args=<optimized out>, ops=<optimized out>, func=<optimized out>) at ../../Src/builtin.c:6408
#7  0x00005643fcba013d in execbuiltin (args=<optimized out>, assigns=<optimized out>, bn=<optimized out>) at ../../Src/builtin.c:507
#8  0x00005643fcbb0b02 in execcmd_exec (state=state@entry=0x7fff40138420, eparams=eparams@entry=0x7fff401351a0, input=input@entry=0, output=output@entry=0, 
    how=<optimized out>, how@entry=18, last1=<optimized out>, last1@entry=2, close_if_forked=-1) at ../../Src/exec.c:4090
#9  0x00005643fcbb105f in execpline2 (state=state@entry=0x7fff40138420, pcode=<optimized out>, how=how@entry=18, input=0, output=0, last1=last1@entry=0)
    at ../../Src/exec.c:1927
#10 0x00005643fcbb1400 in execpline (state=state@entry=0x7fff40138420, slcode=<optimized out>, how=how@entry=18, last1=0) at ../../Src/exec.c:1658
#11 0x00005643fcbb2d60 in execlist (state=state@entry=0x7fff40138420, dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at ../../Src/exec.c:1413
#12 0x00005643fcbd92bc in execif (state=0x7fff40138420, do_exec=0) at ../../Src/loop.c:555
#13 0x00005643fcbaf96c in execcmd_exec (state=state@entry=0x7fff40138420, eparams=eparams@entry=0x7fff40136900, input=input@entry=0, output=output@entry=0, 
    how=<optimized out>, how@entry=18, last1=<optimized out>, last1@entry=2, close_if_forked=-1) at ../../Src/exec.c:3910
#14 0x00005643fcbb105f in execpline2 (state=state@entry=0x7fff40138420, pcode=<optimized out>, how=how@entry=18, input=0, output=0, last1=last1@entry=0)
    at ../../Src/exec.c:1927
#15 0x00005643fcbb1400 in execpline (state=state@entry=0x7fff40138420, slcode=<optimized out>, how=how@entry=18, last1=0) at ../../Src/exec.c:1658
#16 0x00005643fcbb2d60 in execlist (state=state@entry=0x7fff40138420, dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at ../../Src/exec.c:1413
#17 0x00005643fcbd92f8 in execif (state=0x7fff40138420, do_exec=0) at ../../Src/loop.c:576
#18 0x00005643fcbaf96c in execcmd_exec (state=state@entry=0x7fff40138420, eparams=eparams@entry=0x7fff40138060, input=input@entry=0, output=output@entry=0, 
    how=<optimized out>, how@entry=18, last1=<optimized out>, last1@entry=2, close_if_forked=-1) at ../../Src/exec.c:3910
#19 0x00005643fcbb105f in execpline2 (state=state@entry=0x7fff40138420, pcode=<optimized out>, how=how@entry=18, input=0, output=0, last1=last1@entry=0)
    at ../../Src/exec.c:1927
#20 0x00005643fcbb1400 in execpline (state=state@entry=0x7fff40138420, slcode=<optimized out>, how=how@entry=18, last1=0) at ../../Src/exec.c:1658
#21 0x00005643fcbb2d60 in execlist (state=state@entry=0x7fff40138420, dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at ../../Src/exec.c:1413
#22 0x00005643fcbb3202 in execode (p=p@entry=0x5643fd1b2270, dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0, 
    context=context@entry=0x5643fcc24b8d "shfunc") at ../../Src/exec.c:1192
#23 0x00005643fcbb40ab in runshfunc (prog=0x5643fd1b2270, wrap=0x0, name=0x7fb6d8003170 "zshexit") at ../../Src/exec.c:5974
#24 0x00005643fcbb45d6 in doshfunc (shfunc=0x5643fd1ccea0, doshargs=doshargs@entry=0x0, noreturnval=noreturnval@entry=1) at ../../Src/exec.c:5824
#25 0x00005643fcc17a43 in callhookfunc (name=0x5643fcc23818 "zshexit", lnklst=0x0, arrayp=1, retval=0x0) at ../../Src/utils.c:1530
#26 0x00005643fcba6095 in zexit (val=<optimized out>, from_where=<optimized out>) at ../../Src/builtin.c:5810
#27 0x00007fb6d7fcecb4 in getbyte (do_keytmout=do_keytmout@entry=0, timeout=timeout@entry=0x0, full=full@entry=1) at ../../../Src/Zle/zle_main.c:904
#28 0x00007fb6d7fcdbc3 in getkeybuf (w=<optimized out>) at ../../../Src/Zle/zle_keymap.c:1676
#29 getkeymapcmd (km=0x5643fd1900f0, funcp=funcp@entry=0x7fff40138c68, strp=strp@entry=0x7fff40138c70) at ../../../Src/Zle/zle_keymap.c:1587
#30 0x00007fb6d7fcddae in getkeycmd () at ../../../Src/Zle/zle_keymap.c:1715
#31 0x00007fb6d7fcfed5 in zlecore () at ../../../Src/Zle/zle_main.c:1124
#32 0x00007fb6d7fd0dd0 in zleread (lp=<optimized out>, rp=<optimized out>, flags=<optimized out>, context=0, init=0x7fb6d7ff041d "zle-line-init", 
    finish=0x7fb6d7ff040d "zle-line-finish") at ../../../Src/Zle/zle_main.c:1347
#33 0x00005643fcbcbcb8 in zleentry (cmd=cmd@entry=1) at ../../Src/init.c:1604
#34 0x00005643fcbcd209 in inputline () at ../../Src/input.c:295
#35 ingetc () at ../../Src/input.c:228
#36 0x00005643fcbcd37f in ingetc () at ../../Src/input.c:196
#37 0x00005643fcbc4e66 in ihgetc () at ../../Src/hist.c:407
#38 0x00005643fcbd6a9e in gettok () at ../../Src/lex.c:611
#39 zshlex () at ../../Src/lex.c:275
#40 0x00005643fcbd787e in zshlex () at ../../Src/lex.c:268
#41 0x00005643fcbf6776 in parse_event (endtok=endtok@entry=37) at ../../Src/parse.c:581
#42 0x00005643fcbc8545 in loop (toplevel=toplevel@entry=1, justonce=justonce@entry=0) at ../../Src/init.c:147
#43 0x00005643fcbcc2c6 in zsh_main (argc=<optimized out>, argv=<optimized out>) at ../../Src/init.c:1758
#44 0x00007fb6d84da09b in __libc_start_main () from /lib/x86_64-linux-gnu/libc.so.6
#45 0x00005643fcb90d4a in _start ()
```

Looking into `getbyte` in zle_main.c there's this code:

```c
r = raw_getbyte(do_keytmout, &cc, full);
…
if (r == 0) {
    /* The test for IGNOREEOF was added to make zsh ignore ^Ds
       that were typed while commands are running.  Unfortunately
       this caused trouble under at least one system (SunOS 4.1).
       Here shells that lost their xterm (e.g. if it was killed
       with -9) didn't fail to read from the terminal but instead
       happily continued to read EOFs, so that the above read
       returned with 0, and, with IGNOREEOF set, this caused
       an infinite loop.  The simple way around this was to add
       the counter (icnt) so that this happens 20 times and than
       the shell gives up (yes, this is a bit dirty...). */
    if ((zlereadflags & ZLRF_IGNOREEOF) && icnt++ < 20)
        continue;
    stopmsg = 1;
    zexit(1, ZEXIT_NORMAL);
}
```

So, if `raw_getbyte` returns `0` `zexit` gets called again. I think this
causes the loop. I didn't debug a hanging process. I've got the backtrace
from a core dump.

Regards Jörg

-- 
Jeder macht, was er will – Keiner macht was er soll – Aber alle machen mit!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 269 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 10:17 Zsh spins in endless loop with SIGHUP + read in zshexit Jörg Sommer
@ 2021-05-13 14:36 ` Daniel Shahaf
  2021-07-18 19:11   ` Lawrence Velázquez
  2021-05-13 15:51 ` Peter Stephenson
  2021-05-13 15:54 ` Bart Schaefer
  2 siblings, 1 reply; 13+ messages in thread
From: Daniel Shahaf @ 2021-05-13 14:36 UTC (permalink / raw)
  To: Jörg Sommer, zsh-users

Thanks for the bug report and analysis.  I don't have anything to add to that.  However:

Jörg Sommer wrote on Thu, 13 May 2021 10:17 +00:00:
>         if read -rq junk"?Root filesystem is still read-write. Remount ro? (y/N) "

You don't have to name the variable here; «read -q '?foo bar baz '» is valid.  This, then?

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 61dc6986f..92f379d49 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1449,7 +1449,7 @@ findex(read)
 vindex(IFS, use of)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ ))ifnztexi(     )))
 xitem(tt(read )[ tt(-rszpqAclneE) ] [ tt(-t) [ var(num) ] ] [ tt(-k) [ var(num) ] ] [ tt(-d) var(delim) ])
-item(SPACES()[ tt(-u) var(n) ] [ var(name)[tt(?)var(prompt)] ] [ var(name) ...  ])(
+item(SPACES()[ tt(-u) var(n) ] [ [var(name)][tt(?)var(prompt)] ] [ var(name) ...  ])(
 vindex(REPLY, use of)
 vindex(reply, use of)
 Read one line and break it into fields using the characters

(Hope the webmail doesn't munge long lines!)

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 10:17 Zsh spins in endless loop with SIGHUP + read in zshexit Jörg Sommer
  2021-05-13 14:36 ` Daniel Shahaf
@ 2021-05-13 15:51 ` Peter Stephenson
  2021-05-13 15:54 ` Bart Schaefer
  2 siblings, 0 replies; 13+ messages in thread
From: Peter Stephenson @ 2021-05-13 15:51 UTC (permalink / raw)
  To: Jörg Sommer, zsh-users


> On 13 May 2021 at 11:17 Jörg Sommer <joerg@jo-so.de> wrote:
> I'm using `read` in `zshexit` and when my terminal crashes (e.g. XTerm gets
> killed or ssh connection dies) Zsh spins in an endless loop until I kill it
> with SIGKILL (SIGTERM doesn't work).
> 
> ```
> zshexit()
> {
>     if [[ ${#${(M)${(f)"$(</proc/self/mountinfo)"}#*/ / rw,}} == 1 ]]
>     then
>         if read -rq junk"?Root filesystem is still read-write. Remount ro? (y/N) "
>         then
>             echo
>             mount -o remount,ro / || sleep 4
>         else
>             echo
>         fi
>     fi
> }
> ```

Yes, that's a clear bug.  Bart's been mulling over some error handling oddities
down here, not sure if that's one of them.  This is a bit of a special case ---
how about this?  Possibly other calls to zexit() might need similar treatment.

(More zsh-workers this time round, as it turns out.)

Warning: I'm currently in webmailland, too, but the diff is originally from a
proper Linux set up.

pws

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 0561c3b3b..e8d1260b1 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -905,7 +905,8 @@ getbyte(long do_keytmout, int *timeout, int full)
 		if ((zlereadflags & ZLRF_IGNOREEOF) && icnt++ < 20)
 		    continue;
 		stopmsg = 1;
-		zexit(1, ZEXIT_NORMAL);
+		if (zexit(1, ZEXIT_NORMAL))
+		    return lastchar = EOF;
 	    }
 	    icnt = 0;
 	    if (errno == EINTR) {
@@ -928,14 +929,15 @@ getbyte(long do_keytmout, int *timeout, int full)
 	    } else if (errno != 0) {
 		zerr("error on TTY read: %e", errno);
 		stopmsg = 1;
-		zexit(1, ZEXIT_NORMAL);
+		if (zexit(1, ZEXIT_NORMAL))
+		    return lastchar = EOF;
 	    }
 	}
 	if (cc == '\r')		/* undo the exchange of \n and \r determined by */
 	    cc = '\n';		/* zsetterm() */
 	else if (cc == '\n')
 	    cc = '\r';
-
+	
 	ret = STOUC(cc);
     }
     /*
diff --git a/Src/builtin.c b/Src/builtin.c
index b7ceefd55..aefd1436f 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5816,10 +5816,14 @@ _realexit(void)
  *   ZEXIT_DEFERRED if we can't actually exit yet (e.g., functions need
  *                  terminating) but should perform the usual interactive
  *                  tests.
+ *
+ * If this returns 1, we are already exiting the shell:  deeply
+ * embedded calls to this function should give up at that point
+ * and return an error indication.
  */
 
 /**/
-mod_export void
+mod_export int
 zexit(int val, enum zexit_t from_where)
 {
     /*
@@ -5829,7 +5833,7 @@ zexit(int val, enum zexit_t from_where)
      */
     exit_val = val;
     if (shell_exiting == -1)
-	return;
+	return 1;
 
     if (isset(MONITOR) && !stopmsg && from_where != ZEXIT_SIGNAL) {
 	scanjobs();    /* check if jobs need printing           */
@@ -5837,13 +5841,13 @@ zexit(int val, enum zexit_t from_where)
 	    checkjobs();   /* check if any jobs are running/stopped */
 	if (stopmsg) {
 	    stopmsg = 2;
-	    return;
+	    return 0;
 	}
     }
     /* Positive shell_exiting means we have been here before */
     if (from_where == ZEXIT_DEFERRED ||
 	(shell_exiting++ && from_where != ZEXIT_NORMAL))
-	return;
+	return 0;
 
     /*
      * We're now committed to exiting.  Set shell_exiting to -1 to
@@ -5893,6 +5897,8 @@ zexit(int val, enum zexit_t from_where)
 	_exit(exit_val);
     else
 	exit(exit_val);
+    /*NOTREACHED*/ /* I hope */
+    return 0;
 }
 
 /* . (dot), source */


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 10:17 Zsh spins in endless loop with SIGHUP + read in zshexit Jörg Sommer
  2021-05-13 14:36 ` Daniel Shahaf
  2021-05-13 15:51 ` Peter Stephenson
@ 2021-05-13 15:54 ` Bart Schaefer
  2021-05-13 16:07   ` Peter Stephenson
  2 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-05-13 15:54 UTC (permalink / raw)
  To: Jörg Sommer; +Cc: Zsh Users

On Thu, May 13, 2021 at 3:18 AM Jörg Sommer <joerg@jo-so.de> wrote:
>
> I'm using `read` in `zshexit` and when my terminal crashes (e.g. XTerm gets
> killed or ssh connection dies) Zsh spins in an endless loop until I kill it
> with SIGKILL (SIGTERM doesn't work).
> [...]
>
> So, if `raw_getbyte` returns `0` `zexit` gets called again. I think this
> causes the loop.

Yep.  zexit() is a no-op there because it doesn't allow itself to
execute recursively.

There might be other places where similar loops can be triggered.  I
see just now PWS has also produced a different patch, which requires
that zexit() return a value; I have no opinion on whether that's
actually preferable.

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 0561c3b3b..9edf30e01 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -906,6 +906,8 @@ getbyte(long do_keytmout, int *timeout, int full)
             continue;
         stopmsg = 1;
         zexit(1, ZEXIT_NORMAL);
+        /* If called from an exit hook, zexit() returns, so: */
+        break;
         }
         icnt = 0;
         if (errno == EINTR) {
@@ -929,6 +931,8 @@ getbyte(long do_keytmout, int *timeout, int full)
         zerr("error on TTY read: %e", errno);
         stopmsg = 1;
         zexit(1, ZEXIT_NORMAL);
+        /* If called from an exit hook, zexit() returns, so: */
+        break;
         }
     }
     if (cc == '\r')        /* undo the exchange of \n and \r determined by */


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 15:54 ` Bart Schaefer
@ 2021-05-13 16:07   ` Peter Stephenson
  2021-05-13 16:30     ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2021-05-13 16:07 UTC (permalink / raw)
  To: Bart Schaefer, Jörg Sommer; +Cc: Zsh Users

> On 13 May 2021 at 16:54 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Thu, May 13, 2021 at 3:18 AM Jörg Sommer <joerg@jo-so.de> wrote:
> >
> > I'm using `read` in `zshexit` and when my terminal crashes (e.g. XTerm gets
> > killed or ssh connection dies) Zsh spins in an endless loop until I kill it
> > with SIGKILL (SIGTERM doesn't work).
> > [...]
> >
> > So, if `raw_getbyte` returns `0` `zexit` gets called again. I think this
> > causes the loop.
> 
> Yep.  zexit() is a no-op there because it doesn't allow itself to
> execute recursively.
> 
> There might be other places where similar loops can be triggered.  I
> see just now PWS has also produced a different patch, which requires
> that zexit() return a value; I have no opinion on whether that's
> actually preferable.

I think the only possible issue with just a break would be if we didn't
want to return immediately.  But I don't think that's an issue here:
zexit() can't do it's "ooh! you've got jobs" because it's way too late,
and the repeat over multiple ^D's is handled down here in ZLE anyway,
so it's in a position to know that's no-go as well.  So don't see why
your simple patch shouldn't work.

pws


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 16:07   ` Peter Stephenson
@ 2021-05-13 16:30     ` Bart Schaefer
  2021-05-13 18:44       ` Peter Stephenson
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-05-13 16:30 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Jörg Sommer, Zsh Users

On Thu, May 13, 2021 at 9:07 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> [...] don't see why
> your simple patch shouldn't work.

I suppose it comes down to what we might find in other callers of zexit().


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 16:30     ` Bart Schaefer
@ 2021-05-13 18:44       ` Peter Stephenson
  2021-05-13 21:37         ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2021-05-13 18:44 UTC (permalink / raw)
  To: zsh-users

On Thu, 2021-05-13 at 09:30 -0700, Bart Schaefer wrote:
> On Thu, May 13, 2021 at 9:07 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> > 
> > [...] don't see why
> > your simple patch shouldn't work.
> 
> I suppose it comes down to what we might find in other callers of
> zexit().

Presumably ones in loops are the nasty ones.  I couldn't see anything
obvious.

I haven't looked for cases where we're disobeying "exit" from shell code
owing to the same logic, but I think the fix there would probably be
different (you may be stuck in a loop in shell code).  This is
completely hypothetical, though.

pws



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 18:44       ` Peter Stephenson
@ 2021-05-13 21:37         ` Bart Schaefer
  2021-05-14  9:56           ` Peter Stephenson
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-05-13 21:37 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh Users

On Thu, May 13, 2021 at 11:44 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> I haven't looked for cases where we're disobeying "exit" from shell code
> owing to the same logic

So besides the one discussed already in this thread, we have:
1) ${unset?error message}
2) return at top level (bin_break + BIN_RETURN)
3) exit (bin_break + BIN_EXIT)
  3a) in a shell function (ZEXIT_DEFERRED)
  3b) at top level (ZEXIT_NORMAL)
4) doshfunc() when not already in an exit trap
5) loop() when (3a) has previously happened
6) init_misc() for "zsh -c somecommand"
7) zsh_main() at end of input (can be canceled by ignoreeof)
8) zpty module at the end of the child thread
9) signal handling for HUP PIPE ALRM and if not interactive INT

(3a) uses zexit in a comma expression, I'm surprised that doesn't
produce a warning for a void function.  That is the only instance of
DEFERRED.

The only time zexit() fails to _exit() is when called recursively,
which I believe can only happen in a trap.  Possible "dangerous" cases
when used in a trap:

(1) could unexpectedly proceed beyond the parameter expansion into the
subsequent shell code.
(2,3) and by extension (4) when used in combination with an "always"
block (this is not clear-cut) would continue the shell code.
(8) would continue into the pty master code if a new zpty were opened.

I haven't tested whether any of these things actually DO fail to
_exit(), just source code examination of what comes downstream in each
circumstance.  When there is no "always", I believe 2 and 3 correctly
end processing of the current function scope.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 21:37         ` Bart Schaefer
@ 2021-05-14  9:56           ` Peter Stephenson
  2021-05-14 17:44             ` Peter Stephenson
  2021-05-14 18:51             ` Bart Schaefer
  0 siblings, 2 replies; 13+ messages in thread
From: Peter Stephenson @ 2021-05-14  9:56 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh Users


> On 13 May 2021 at 22:37 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Thu, May 13, 2021 at 11:44 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > I haven't looked for cases where we're disobeying "exit" from shell code
> > owing to the same logic
> 
> So besides the one discussed already in this thread, we have:
> 1) ${unset?error message}
> 2) return at top level (bin_break + BIN_RETURN)
> 3) exit (bin_break + BIN_EXIT)
>   3a) in a shell function (ZEXIT_DEFERRED)

Focussing on this one, I tried

TRAPEXIT() { while true; do exit; done; }

and sure enough the shell is looping forever if I kill the terminal.
(Obviously, as written that's a daft thing to do, but with an appropriate
set of internal conditions it might become realistic.)

This is the sort of thing I was thinking about --- the answer is probably
going to be along the lines of "breaks = loops" etc. in that condition
at the top of zexit(), but I haven't made a patch as I suspect there's
going to be more to the can of worms.

pws

>   3b) at top level (ZEXIT_NORMAL)
> 4) doshfunc() when not already in an exit trap
> 5) loop() when (3a) has previously happened
> 6) init_misc() for "zsh -c somecommand"
> 7) zsh_main() at end of input (can be canceled by ignoreeof)
> 8) zpty module at the end of the child thread
> 9) signal handling for HUP PIPE ALRM and if not interactive INT
> 
> (3a) uses zexit in a comma expression, I'm surprised that doesn't
> produce a warning for a void function.  That is the only instance of
> DEFERRED.
> 
> The only time zexit() fails to _exit() is when called recursively,
> which I believe can only happen in a trap.  Possible "dangerous" cases
> when used in a trap:
> 
> (1) could unexpectedly proceed beyond the parameter expansion into the
> subsequent shell code.
> (2,3) and by extension (4) when used in combination with an "always"
> block (this is not clear-cut) would continue the shell code.
> (8) would continue into the pty master code if a new zpty were opened.
> 
> I haven't tested whether any of these things actually DO fail to
> _exit(), just source code examination of what comes downstream in each
> circumstance.  When there is no "always", I believe 2 and 3 correctly
> end processing of the current function scope.
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-14  9:56           ` Peter Stephenson
@ 2021-05-14 17:44             ` Peter Stephenson
  2021-05-14 18:51             ` Bart Schaefer
  1 sibling, 0 replies; 13+ messages in thread
From: Peter Stephenson @ 2021-05-14 17:44 UTC (permalink / raw)
  To: zsh-users

On Fri, 2021-05-14 at 10:56 +0100, Peter Stephenson wrote:
> > 3) exit (bin_break + BIN_EXIT)
> >   3a) in a shell function (ZEXIT_DEFERRED)
> 
> Focussing on this one, I tried
> 
> TRAPEXIT() { while true; do exit; done; }
> 
> and sure enough the shell is looping forever if I kill the terminal.
> (Obviously, as written that's a daft thing to do, but with an appropriate
> set of internal conditions it might become realistic.)
> 
> This is the sort of thing I was thinking about --- the answer is probably
> going to be along the lines of "breaks = loops" etc. in that condition
> at the top of zexit(), but I haven't made a patch as I suspect there's
> going to be more to the can of worms.

Actually, this seems to work and may be one of the less complicated
aspects of this.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index b7ceefd55..a29eb49e4 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5828,8 +5828,11 @@ zexit(int val, enum zexit_t from_where)
      * a later value always overrides an earlier.
      */
     exit_val = val;
-    if (shell_exiting == -1)
+    if (shell_exiting == -1) {
+	retflag = 1;
+	breaks = loops;
 	return;
+    }
 
     if (isset(MONITOR) && !stopmsg && from_where != ZEXIT_SIGNAL) {
 	scanjobs();    /* check if jobs need printing           */



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-14  9:56           ` Peter Stephenson
  2021-05-14 17:44             ` Peter Stephenson
@ 2021-05-14 18:51             ` Bart Schaefer
  1 sibling, 0 replies; 13+ messages in thread
From: Bart Schaefer @ 2021-05-14 18:51 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh Users

On Fri, May 14, 2021 at 2:56 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> >   3a) in a shell function (ZEXIT_DEFERRED)
>
> Focussing on this one, I tried
>
> TRAPEXIT() { while true; do exit; done; }
>
> and sure enough the shell is looping forever if I kill the terminal.

Of course there's nothing preventing someone from writing an infinite
loop in zshexit or TRAPEXIT, but it's reasonable to expect that exit
does stop a loop.

Are you going to commit your patch from users/26742 or shall I sweep
it up along with mine?

Meanwhile we should probably move this thread to -workers.  I hadn't
noticed it was on -users until just now.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-05-13 14:36 ` Daniel Shahaf
@ 2021-07-18 19:11   ` Lawrence Velázquez
  2021-07-27  3:14     ` Daniel Shahaf
  0 siblings, 1 reply; 13+ messages in thread
From: Lawrence Velázquez @ 2021-07-18 19:11 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-users

On Thu, May 13, 2021, at 10:36 AM, Daniel Shahaf wrote:
> Thanks for the bug report and analysis.  I don't have anything to add 
> to that.  However:
> 
> Jörg Sommer wrote on Thu, 13 May 2021 10:17 +00:00:
> >         if read -rq junk"?Root filesystem is still read-write. Remount ro? (y/N) "
> 
> You don't have to name the variable here; «read -q '?foo bar baz '» is 
> valid.  This, then?
> 
> diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
> index 61dc6986f..92f379d49 100644
> --- a/Doc/Zsh/builtins.yo
> +++ b/Doc/Zsh/builtins.yo
> @@ -1449,7 +1449,7 @@ findex(read)
>  vindex(IFS, use of)
>  redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ ))ifnztexi(     )))
>  xitem(tt(read )[ tt(-rszpqAclneE) ] [ tt(-t) [ var(num) ] ] [ tt(-k) [ 
> var(num) ] ] [ tt(-d) var(delim) ])
> -item(SPACES()[ tt(-u) var(n) ] [ var(name)[tt(?)var(prompt)] ] [ 
> var(name) ...  ])(
> +item(SPACES()[ tt(-u) var(n) ] [ [var(name)][tt(?)var(prompt)] ] [ 
> var(name) ...  ])(
>  vindex(REPLY, use of)
>  vindex(reply, use of)
>  Read one line and break it into fields using the characters

Hi Daniel,

Planning on committing this?

-- 
vq


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Zsh spins in endless loop with SIGHUP + read in zshexit
  2021-07-18 19:11   ` Lawrence Velázquez
@ 2021-07-27  3:14     ` Daniel Shahaf
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Shahaf @ 2021-07-27  3:14 UTC (permalink / raw)
  To: Lawrence Velázquez; +Cc: zsh-users, Daniel Shahaf

Lawrence Velázquez wrote on Sun, Jul 18, 2021 at 15:11:42 -0400:
> On Thu, May 13, 2021, at 10:36 AM, Daniel Shahaf wrote:
> > Thanks for the bug report and analysis.  I don't have anything to add 
> > to that.  However:
> > 
> > Jörg Sommer wrote on Thu, 13 May 2021 10:17 +00:00:
> > >         if read -rq junk"?Root filesystem is still read-write. Remount ro? (y/N) "
> > 
> > You don't have to name the variable here; «read -q '?foo bar baz '» is 
> > valid.  This, then?
> > 
> > diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
> > index 61dc6986f..92f379d49 100644
> > --- a/Doc/Zsh/builtins.yo
> > +++ b/Doc/Zsh/builtins.yo
> > @@ -1449,7 +1449,7 @@ findex(read)
> >  vindex(IFS, use of)
> >  redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ ))ifnztexi(     )))
> >  xitem(tt(read )[ tt(-rszpqAclneE) ] [ tt(-t) [ var(num) ] ] [ tt(-k) [ 
> > var(num) ] ] [ tt(-d) var(delim) ])
> > -item(SPACES()[ tt(-u) var(n) ] [ var(name)[tt(?)var(prompt)] ] [ 
> > var(name) ...  ])(
> > +item(SPACES()[ tt(-u) var(n) ] [ [var(name)][tt(?)var(prompt)] ] [ 
> > var(name) ...  ])(
> >  vindex(REPLY, use of)
> >  vindex(reply, use of)
> >  Read one line and break it into fields using the characters
> 
> Hi Daniel,
> 
> Planning on committing this?

Yes.  Haven't had time to do so yet.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-07-27  3:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-13 10:17 Zsh spins in endless loop with SIGHUP + read in zshexit Jörg Sommer
2021-05-13 14:36 ` Daniel Shahaf
2021-07-18 19:11   ` Lawrence Velázquez
2021-07-27  3:14     ` Daniel Shahaf
2021-05-13 15:51 ` Peter Stephenson
2021-05-13 15:54 ` Bart Schaefer
2021-05-13 16:07   ` Peter Stephenson
2021-05-13 16:30     ` Bart Schaefer
2021-05-13 18:44       ` Peter Stephenson
2021-05-13 21:37         ` Bart Schaefer
2021-05-14  9:56           ` Peter Stephenson
2021-05-14 17:44             ` Peter Stephenson
2021-05-14 18:51             ` 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).