zsh-workers/9267

Src/signals.c

@@ -635,11 +635,6 @@ struct savetrap {
 
 static LinkList savetraps;
 
-/* Flag to unsettrap not to free the structs, which we're keeping */
-
-/**/
-int notrapfree;
-
 /*
  * Save the current trap and unset it.
  */
@@ -651,7 +646,6 @@ dosavetrap(int sig, int level)
     st = (struct savetrap *)zalloc(sizeof(*st));
     st->sig = sig;
     st->local = level;
-    notrapfree++;
     if ((st->flags = sigtrapped[sig]) & ZSIG_FUNC) {
 	/*
 	 * Get the old function: this assumes we haven't added
@@ -667,10 +661,7 @@ dosavetrap(int sig, int level)
     } else {
 	st->list = sigfuncs[sig];
 	sigfuncs[sig] = NULL;
-	unsettrap(sig);
     }
-    sigtrapped[sig] = 0;
-    notrapfree--;
     PERMALLOC {
 	if (!savetraps)
 	    savetraps = newlinklist();
@@ -691,16 +682,13 @@ settrap(int sig, List l)
         zerr("can't trap SIG%s in interactive shells", sigs[sig], 0);
         return 1;
     }
+
     /*
-     * Note that we save the trap here even if there isn't an existing
+     * Call unsettrap() unconditionally, to make sure trap is saved
-     * one, to aid in removing this one.  However, if there's
+     * if necessary.
-     * already one at the current locallevel we just overwrite it.
      */
-    if (isset(LOCALTRAPS) && locallevel &&
+    unsettrap(sig);
-	(!sigtrapped[sig] || locallevel > (sigtrapped[sig] >> ZSIG_SHIFT))) {
+
-	dosavetrap(sig, locallevel);
-    } else if (sigfuncs[sig])
-	unsettrap(sig);
     sigfuncs[sig] = l;
     if (!l) {
 	sigtrapped[sig] = ZSIG_IGNORED;
@@ -734,23 +722,23 @@ unsettrap(int sig)
 {
     int trapped;
 
-    if (sig == -1 || !(trapped = sigtrapped[sig]) ||
+    if (sig == -1 ||
-	(jobbing && (sig == SIGTTOU || sig == SIGTSTP || sig == SIGTTIN))) {
+	(jobbing && (sig == SIGTTOU || sig == SIGTSTP || sig == SIGTTIN)))
-        return;
-    }
-    if (isset(LOCALTRAPS) && locallevel &&
-	sigtrapped[sig] && locallevel > (sigtrapped[sig] >> ZSIG_SHIFT)) {
-	/*
-	 * This calls unsettrap recursively to do any dirty work, so
-	 * make sure this bit doesn't happen:  a bit messy, but hard
-	 * to avoid.
-	 */
-	int oldlt = opts[LOCALTRAPS];
-	opts[LOCALTRAPS] = 0;
-	dosavetrap(sig, locallevel);
-	opts[LOCALTRAPS] = oldlt;
 	return;
-    }
+
+    trapped = sigtrapped[sig];
+    /*
+     * Note that we save the trap here even if there isn't an existing
+     * one, to aid in removing this one.  However, if there's
+     * already one at the current locallevel we just overwrite it.
+     */
+    if (isset(LOCALTRAPS) && locallevel &&
+	(!trapped || locallevel > (sigtrapped[sig] >> ZSIG_SHIFT)))
+	dosavetrap(sig, locallevel);
+
+    if (!trapped)
+        return;
+
     sigtrapped[sig] = 0;
     if (sig == SIGINT && interact) {
 	/* PWS 1995/05/16:  added test for interactive, also noholdintr() *
@@ -765,14 +753,24 @@ unsettrap(int sig)
 #endif
              sig != SIGCHLD)
         signal_default(sig);
-    if (notrapfree)
+
-	return;
+    /*
+     * At this point we free the appropriate structs.  If we don't
+     * want that to happen (e.g. we are saving the trap), then
+     * either the function should already have been removed from shfunctab,
+     * or the entry in sigfuncs should have been set to NULL, and then
+     * we're laughing, in a sort of vague virtual sense.
+     */
     if (trapped & ZSIG_FUNC) {
 	char func[20];
 	HashNode hn;
 
 	sprintf(func, "TRAP%s", sigs[sig]);
-	if ((hn = shfunctab->removenode(shfunctab, func)))
+	/*
+	 * As in dosavetrap(), don't call removeshfuncnode() because
+	 * that calls back into unsettrap();
+	 */
+	if ((hn = removehashnode(shfunctab, func)))
 	    shfunctab->freenode(hn);
     } else if (sigfuncs[sig]) {
 	freestruct(sigfuncs[sig]);
@@ -786,10 +784,14 @@ starttrapscope(void)
 {
     /*
      * SIGEXIT needs to be restored at the current locallevel,
-     * so give it the next higher one.
+     * so give it the next higher one. dosavetrap() is called
+     * automatically where necessary.
      */
-    if (sigtrapped[SIGEXIT])
+    if (sigtrapped[SIGEXIT]) {
-	dosavetrap(SIGEXIT, locallevel+1);
+	locallevel++;
+	unsettrap(SIGEXIT);
+	locallevel--;
+    }
 }
 
 /*
@@ -811,14 +813,13 @@ endtrapscope(void)
      * after all the other traps have been put back.
      */
     if ((exittr = sigtrapped[SIGEXIT])) {
-	notrapfree++;
 	if (exittr & ZSIG_FUNC) {
-	    exitfn = shfunctab->removenode(shfunctab, "TRAPEXIT");
+	    exitfn = removehashnode(shfunctab, "TRAPEXIT");
 	} else {
 	    exitfn = sigfuncs[SIGEXIT];
-	    unsettrap(SIGEXIT);
+	    sigfuncs[SIGEXIT] = NULL;
 	}
-	notrapfree--;
+	unsettrap(SIGEXIT);
     }
 
     if (savetraps) {

Test/08traps.ztst

@@ -0,0 +1,128 @@
+# Tests for both trap builtin and TRAP* functions.
+
+%prep
+
+  setopt localtraps
+  mkdir traps.tmp && cd traps.tmp
+
+%test
+
+  fn1() {
+    trap 'print EXIT1' EXIT
+    fn2() { trap 'print EXIT2' EXIT; }
+    fn2
+  }
+  fn1
+0:Nested `trap ... EXIT'
+>EXIT2
+>EXIT1
+
+  fn1() {
+    TRAPEXIT() { print EXIT1; }
+    fn2() { TRAPEXIT() { print EXIT2; }; }
+    fn2
+  }
+  fn1
+0: Nested TRAPEXIT
+>EXIT2
+>EXIT1
+
+  fn1() {
+    trap 'print EXIT1' EXIT
+    fn2() { trap - EXIT; }
+    fn2
+  }
+  fn1
+0:Nested `trap - EXIT' on `trap ... EXIT'
+>EXIT1
+
+  fn1() {
+    TRAPEXIT() { print EXIT1; }
+    fn2() { trap - EXIT; }
+    fn2
+  }
+  fn1
+0:Nested `trap - EXIT' on `TRAPEXIT'
+>EXIT1
+
+  fn1() {
+    trap
+    trap 'print INT1' INT
+    fn2() { trap 'print INT2' INT; trap; }
+    trap
+    fn2
+    trap
+  }
+  fn1
+0: Nested `trap ... INT', not triggered
+>trap -- 'print INT1' INT
+>trap -- 'print INT2' INT
+>trap -- 'print INT1' INT
+
+   fn1() {
+    trap
+    TRAPINT() { print INT1; }
+    fn2() { TRAPINT() { print INT2; }; trap; }
+    trap
+    fn2
+    trap
+  }
+  fn1
+0: Nested `trap ... INT', not triggered
+>TRAPINT () {
+>	print INT1
+>}
+>TRAPINT () {
+>	print INT2
+>}
+>TRAPINT () {
+>	print INT1
+>}
+
+  fn1() {
+    trap 'print INT1' INT
+    fn2() { trap - INT; trap; }
+    trap
+    fn2
+    trap
+  }
+  fn1
+0: Nested `trap - INT' on untriggered `trap ... INT'
+>trap -- 'print INT1' INT
+>trap -- 'print INT1' INT
+
+# Testing the triggering of traps here is very unpleasant.
+# The delays are attempts to avoid race conditions, though there is
+# no guarantee that they will work.  Note the subtlety that the
+# `sleep' in the function which receives the trap does *not* get the
+# signal, only the parent shell, which is waiting for a SIGCHILD.
+# (At least, that's what I think is happening.) Thus we have to wait at
+# least the full two seconds to make sure we have got the output from the
+# execution of the trap.
+
+  print 'This test takes at least three seconds...' >&8
+  fn1() {
+    trap 'print TERM1' TERM
+    fn2() { trap 'print TERM2; return 1' TERM; sleep 2; }
+    fn2 &
+    sleep 1
+    kill -TERM $!
+    sleep 2
+  }
+  fn1
+0: Nested `trap ... TERM', triggered on inner loop
+>TERM2
+
+  print 'This test, too, takes at least three seconds...' >&8
+  fn1() {
+    trap 'print TERM1; return 1' TERM
+    fn2() { trap 'print TERM2; return 1' TERM; }
+    fn2
+    sleep 2
+  }
+  fn1 &
+  sleep 1
+  kill -TERM $!
+  sleep 2
+0: Nested `trap ... TERM', triggered on outer loop
+>TERM1

Test/50cd.ztst

@@ -17,11 +17,10 @@
 # of spaces and/or tabs, to differentiate it from tags with a special
 # meaning to the test harness.  Note that this is true even in sections
 # where there are no such tags.  Also note that file descriptor 9
-# is reserved for input from the test script; if ZTST_verbose is set,
+# is reserved for input from the test script, and file descriptor 8
-# output is sent to the original stdout via fd 8.  Option settings
+# preserves the original stdout.  Option settings are preserved between the
-# are preserved between the execution of different code chunks;
+# execution of different code chunks; initially, all standard zsh options
-# initially, all standard zsh options (the effect of `emulate -R zsh')
+# (the effect of `emulate -R zsh') are set.
-# are set.
 
 %prep
 # This optional section prepares the test, creating directories and files