Re: Call stack issues when running trap handler

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Fri, 21 Apr 2017 22:32:46 +0200
Sebastian Reuße <seb@xxxxxxxxxxx> wrote:
> I’ve noticed that under certain conditions, shell functions called from
> inside an exit trap handler appear to never return. E.g., running the
> following code will only yield «a» as output, indicating that callee
> «echoa» never returns control to the caller «handler».

I've just re-read this and compared with the evidence and obviously I've
misinterpreted it.  When you said "never return", you meant it exited
from that point (so never printed "b" but did leave the shell
nonetheless).  I interpreted this as saying it got stuck in that
function, but that obviously isn't what you mean.

This I can fix.

If anyone knows whether I need to distinguish in any of the following
between EXIT traps for functions and the shell as a whole, feel free to
tell me.  I'm assuming it's going to be too arcane to worry about.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index b2e552d..ff07b04 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5509,8 +5509,11 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
 	     *
 	     * If we are forked, we exit the shell at the function depth
 	     * at which we became a subshell, hence the comparison.
+	     *
+	     * If we *are* in an EXIT trap... give this all up as
+	     * a bad job.
 	     */
-	    if (stopmsg || (zexit(0,2), !stopmsg)) {
+	    if ((stopmsg || (zexit(0,2), !stopmsg)) && !in_exit_trap) {
 		retflag = 1;
 		breaks = loops;
 		exit_pending = (num << 1) | 1;
diff --git a/Src/exec.c b/Src/exec.c
index 978a32d..e0fc544 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5688,8 +5688,11 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
      * the only likely case where we need that second test is
      * when we have an "always" block.  The endparamscope() has
      * already happened, hence the "+1" here.
+     *
+     * If we are in an exit trap, finish it first... we wouldn't set
+     * exit_pending if we were already in one.
      */
-    if (exit_pending && exit_level >= locallevel+1) {
+    if (exit_pending && exit_level >= locallevel+1 && !in_exit_trap) {
 	if (locallevel > forklevel) {
 	    /* Still functions to return: force them to do so. */
 	    retflag = 1;
diff --git a/Src/signals.c b/Src/signals.c
index 68a7ae3..cad40f4 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -55,6 +55,11 @@ mod_export Eprog siglists[VSIGCOUNT];
 /**/
 mod_export int nsigtrapped;
 
+/* Running an exit trap? */
+
+/**/
+int in_exit_trap;
+
 /*
  * Flag that exit trap has been set in POSIX mode.
  * The setter's expectation is therefore that it is run
@@ -1435,7 +1440,13 @@ dotrap(int sig)
 
     dont_queue_signals();
 
+    if (sig == SIGEXIT)
+	++in_exit_trap;
+
     dotrapargs(sig, sigtrapped+sig, funcprog);
 
+    if (sig == SIGEXIT)
+	--in_exit_trap;
+
     restore_queue_signals(q);
 }