Re: [Bug] Exiting shell from function called by trap handler always produces status 0

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Wed, 2018-10-10 at 08:56 +0000, Daniel Shahaf wrote:
> Peter Stephenson wrote on Tue, 09 Oct 2018 21:09 +0100:
> > 
> > Here's a slight improvement --- we can "exit 0" even if the last command
> > status is non-zero.
> Can we add a test for this?  I tried to write one but couldn't nail down
> the behaviour that differed.
> 
> FWIW:
> 
> % Src/zsh -fc 'fn() { exit 0 }; trap fn EXIT; false'  ; echo $?
> 1
> % /usr/bin/zsh -fc 'fn() { exit 0 }; trap fn EXIT; false' ; echo $?
> 0
> % 

I missed another case for implicit exit that was masking the effect.  We
go through zexit() at this point as it's a standard end-of-command-list
case, though whether that's actually necessary (running additional tests
mostly associated with interactive shells) I don't know.

It's also a bit confusing that the exit trap has already happened in
that point --- in execlist(), I think --- even though it's more sensibly
associated in this case with the code inside zexit() which will also run
it (and if it had done the exit status would have been overridden and we
wouldn't have seen this).  But I don't think there are any real world
consequences for this with this change.

I don't think an exit_pending can actually be picked up at that point,
but it's easier and safer to be consistent.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index e01e035..8dcdcc0 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5723,7 +5723,7 @@ int exit_val;
 void
 realexit(void)
 {
-    exit(exit_val ? exit_val : lastval);
+    exit((shell_exiting || exit_pending) ? exit_val : lastval);
 }
 
 /* As realexit(), but call _exit instead */
@@ -5732,7 +5732,7 @@ realexit(void)
 void
 _realexit(void)
 {
-    _exit(exit_val ? exit_val : lastval);
+    _exit((shell_exiting || exit_pending) ? exit_val : lastval);
 }
 
 /* exit the shell.  val is the return value of the shell.  *
diff --git a/Src/init.c b/Src/init.c
index 838c2c2..cec9143 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1358,7 +1358,7 @@ init_misc(char *cmd, char *zsh_name)
 	bshin = fdopen(SHIN, "r");
 	execstring(cmd, 0, 1, "cmdarg");
 	stopmsg = 1;
-	zexit(exit_val ? exit_val : lastval, 0);
+	zexit((exit_pending || shell_exiting) ? exit_val : lastval, 0);
     }
 
     if (interact && isset(RCS))
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index bab0b0a..57daf8d 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -872,6 +872,9 @@ F:Must be tested with a top-level script rather than source or function
   $ZTST_testdir/../Src/zsh -fc 'fn() { exit 13; }; trap fn EXIT'
 13:Explicit exit in exit trap overrides implicit exit status
 
+  $ZTST_testdir/../Src/zsh -fc 'fn() { exit 0; }; trap fn EXIT; false'
+0:Explicit exit status 0 in exit trap overrides implicit non-zero status
+
 %clean
 
   rm -f TRAPEXIT