zsh-workers
 help / color / mirror / code / Atom feed
From: Bart Schaefer <schaefer@brasslantern.com>
To: Greg Klanderman <gak@klanderman.net>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: using trap function to cleanup and exit?
Date: Thu, 14 Apr 2022 14:29:42 -0700	[thread overview]
Message-ID: <CAH+w=7aEJd_wzJPsBUaDg++UWZLirOiEFpxYodJGbFZc=sHZGA@mail.gmail.com> (raw)
In-Reply-To: <871qy0yzp4.fsf@lwm.klanderman.net>

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

On Wed, Apr 13, 2022 at 10:58 PM Greg Klanderman <gak@klanderman.net> wrote:
>
> The script I posted is using /bin/zsh -f, so TRAPS_ASYNC should be
> off, but I do see a trap run immediately, when the script is running
>
> <external-program> | <shell-function>
>
> at the time the signal is received.
>
> Is the <external-program> considered a foreground child?

No, it's in the background; zsh "forks to the left" so that the
<shell-function> is running in the current shell.  This is the reverse
of bash/ksh.  (In recent zsh, when emulating sh, both sides of the
pipe may be forked.)

> When TRAPTERM uses 'exit' rather than 'return', the script does not
> exit after receiving SIGTERM.

OK, this is indeed a bug, previously discussed in workers/44007 and
follow-ups.  More on this below.

> With 'return', the script does exit,
> but the <external-program> in
>
> <external-program> | <shell-function>
>
> remains running.

This is actually expected, I believe; as noted above,
<external-program> is a background job here, and it would normally be
expected to exit on SIGPIPE when the <shell-function> exits and closes
the connection.  It never gets that signal because it never writes
anything on the pipe.

Regarding workers/44007:  This seems ultimately to be a side-effect
described by this comment:
             * We don't exit directly from functions to allow tidying
             * up, in particular EXIT traps.  We still need to perform
             * the usual interactive tests to see if we can exit at
             * all, however.

In bin_break(), which handles both "return" and "exit", the conditions
applied upon "return" are not the same as those applied upon "exit".
With the builtin.c hunk of the patch below "make check" reports:

Test ./C03traps.ztst was expected to fail, but passed.
Was testing: (workers/44007) function execution continues after 'exit' in trap

I've therefore included cleaning up the BUGS file and reversing the
sense of that test, but would prefer that another set of eyes review
the code change.

[-- Attachment #2: patch44007.txt --]
[-- Type: text/plain, Size: 1541 bytes --]

diff --git a/Etc/BUGS b/Etc/BUGS
index 5624fb24d..3121fc9fa 100644
--- a/Etc/BUGS
+++ b/Etc/BUGS
@@ -30,9 +30,6 @@ skipped when STTY=... is set for that command
 ------------------------------------------------------------------------
 42609: :|: =(hang)
 ------------------------------------------------------------------------
-44007 - Martijn - exit in trap executes rest of function
-See test case in Test/C03traps.ztst.
-------------------------------------------------------------------------
 44133 debian #924736 (partial patch in 44134) three setopts following `    #`
 ------------------------------------------------------------------------
 44850 terminal issues with continuation markers
diff --git a/Src/builtin.c b/Src/builtin.c
index 8ef678b22..b93466ba5 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5720,6 +5720,8 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
 	     * a bad job.
 	     */
 	    if (stopmsg || (zexit(0, ZEXIT_DEFERRED), !stopmsg)) {
+		if (trap_state) 
+		    trap_state = TRAP_STATE_FORCE_RETURN;
 		retflag = 1;
 		breaks = loops;
 		exit_pending = 1;
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index 6f84e5db2..3bd2958cb 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -901,7 +901,7 @@ F:Must be tested with a top-level script rather than source or function
  fn trap1 trap2
  echo out2
  '
--f:(workers/44007) function execution continues after 'exit' in trap
+-:(workers/44007) function execution continues after 'exit' in trap
 >out1
 >fn1
 >trap1


  reply	other threads:[~2022-04-14 21:30 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-10 15:46 Greg Klanderman
2022-04-10 15:57 ` Peter Stephenson
2022-04-10 16:12   ` Greg Klanderman
2022-04-10 16:30     ` Greg Klanderman
2022-04-10 18:15       ` Lawrence Velázquez
2022-04-14  5:13         ` Greg Klanderman
2022-04-10 20:49       ` Bart Schaefer
2022-04-10 20:55         ` Bart Schaefer
2022-04-14  5:57         ` Greg Klanderman
2022-04-14 21:29           ` Bart Schaefer [this message]
2022-04-14 23:35             ` Bart Schaefer
2022-04-17 17:08               ` Daniel Shahaf
2022-04-15 22:27             ` Bart Schaefer
2022-04-19 18:42               ` Peter Stephenson
2022-04-19 18:55                 ` Peter Stephenson
2022-04-19 21:22                   ` Bart Schaefer
2022-04-20  8:16                     ` Peter Stephenson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAH+w=7aEJd_wzJPsBUaDg++UWZLirOiEFpxYodJGbFZc=sHZGA@mail.gmail.com' \
    --to=schaefer@brasslantern.com \
    --cc=gak@klanderman.net \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).