Re: using trap function to cleanup and exit?

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Wed, Apr 13, 2022 at 10:58 PM Greg Klanderman <gak@xxxxxxxxxxxxxx> 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.

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