Skip to content

Commit dc7a405

Browse files
committed
Fix EXIT trap not firing when ERR_EXIT or ${var:?} triggers inside a function
When ERR_EXIT triggers inside a function, the errexit code path called realexit() directly, bypassing endtrapscope() and skipping any EXIT traps saved by starttrapscope() on function entry. Similarly, when ${var:?message} triggers inside a function, the paramsubst code path called _exit(1) or zexit() directly, bypassing the function unwind and skipping EXIT traps at outer scopes. Reuse the same exit_pending unwind mechanism as the exit builtin, so that endtrapscope() runs at each function level and all nested EXIT traps are executed bottom-up, matching the behaviour of an explicit exit call. Extract set_exit_pending() to consolidate the repeated unwind setup (trap_state, retflag, breaks, exit_pending, exit_level, exit_val) used by the exit builtin, the ERR_EXIT handler, and the new ${var:?} handler. Extract restore_saved_trap() from endtrapscope() to reduce duplication in the trap restoration logic. Clear errflag in endtrapscope() when exit_pending, so that EXIT traps can fire even when the exit was triggered by an error (e.g. ${var:?}). This mirrors what zexit() already does before calling dotrap(SIGEXIT).
1 parent 99f5788 commit dc7a405

File tree

6 files changed

+137
-37
lines changed

6 files changed

+137
-37
lines changed

README

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,17 @@ The ERR_EXIT and ERR_RETURN options were refined to be more self-
5656
consistent and better aligned with the POSIX-2017 specification of
5757
`set -e`:
5858

59+
- The EXIT trap is now correctly executed when ERR_EXIT triggers
60+
inside a function. Previously, `starttrapscope()` hid the EXIT
61+
trap on function entry, and the ERR_EXIT code path called
62+
`realexit()` without restoring it. Example:
63+
64+
setopt ERR_EXIT
65+
trap 'echo exiting' EXIT
66+
f() { false }
67+
f
68+
# "exiting" is printed only since 5.10.
69+
5970
- Function calls or anonymous functions prefixed with `!` now never
6071
trigger exit or return. Negated function calls or anonymous
6172
functions used to trigger exit or return if ERR_EXIT or ERR_RETURN

Src/builtin.c

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5823,6 +5823,25 @@ mod_export volatile int exit_pending;
58235823
/**/
58245824
mod_export volatile int exit_level;
58255825

5826+
/*
5827+
* Set up exit_pending unwind: instead of exiting immediately,
5828+
* force all active functions to return so that endtrapscope()
5829+
* runs at each level and all saved EXIT traps are executed.
5830+
*/
5831+
5832+
/**/
5833+
mod_export void
5834+
set_exit_pending(int val)
5835+
{
5836+
if (trap_state)
5837+
trap_state = TRAP_STATE_FORCE_RETURN;
5838+
retflag = 1;
5839+
breaks = loops;
5840+
exit_pending = 1;
5841+
exit_level = locallevel;
5842+
exit_val = val;
5843+
}
5844+
58265845
/* we have printed a 'you have stopped (running) jobs.' message */
58275846

58285847
/**/
@@ -5905,13 +5924,7 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
59055924
* a bad job.
59065925
*/
59075926
if (stopmsg || (zexit(0, ZEXIT_DEFERRED), !stopmsg)) {
5908-
if (trap_state)
5909-
trap_state = TRAP_STATE_FORCE_RETURN;
5910-
retflag = 1;
5911-
breaks = loops;
5912-
exit_pending = 1;
5913-
exit_level = locallevel;
5914-
exit_val = num;
5927+
set_exit_pending(num);
59155928
}
59165929
} else
59175930
zexit(num, ZEXIT_NORMAL);

Src/exec.c

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,12 +1616,22 @@ execlist(Estate state, int dont_change_job, int exiting)
16161616
!(noerrexit & NOERREXIT_EXIT);
16171617
if (errexit) {
16181618
errflag = 0;
1619-
if (sigtrapped[SIGEXIT])
1620-
dotrap(SIGEXIT);
1621-
if (mypid != getpid())
1622-
_realexit();
1623-
else
1624-
realexit();
1619+
if (locallevel > forklevel) {
1620+
/*
1621+
* Inside a function: use the same unwind
1622+
* mechanism as the exit builtin so that
1623+
* endtrapscope() runs at each level and
1624+
* all saved EXIT traps are executed.
1625+
*/
1626+
set_exit_pending(lastval);
1627+
} else {
1628+
if (sigtrapped[SIGEXIT])
1629+
dotrap(SIGEXIT);
1630+
if (mypid != getpid())
1631+
_realexit();
1632+
else
1633+
realexit();
1634+
}
16251635
}
16261636
if (errreturn) {
16271637
retflag = 1;

Src/signals.c

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,36 @@ starttrapscope(void)
875875
* endparamscope() so that the locallevel has been decremented.
876876
*/
877877

878+
/*
879+
* Restore a single saved trap entry: call settrap() to reinstall it,
880+
* update sigtrapped[], and for function-style traps re-add the node
881+
* to shfunctab. Used by endtrapscope().
882+
*/
883+
884+
static void
885+
restore_saved_trap(struct savetrap *st)
886+
{
887+
int sig = st->sig;
888+
889+
dontsavetrap++;
890+
if (st->flags & ZSIG_FUNC)
891+
settrap(sig, NULL, ZSIG_FUNC);
892+
else
893+
settrap(sig, (Eprog) st->list, 0);
894+
if (sig == SIGEXIT)
895+
exit_trap_posix = st->posix;
896+
dontsavetrap--;
897+
/*
898+
* counting of nsigtrapped should presumably be handled
899+
* in settrap...
900+
*/
901+
DPUTS((sigtrapped[sig] ^ st->flags) & ZSIG_TRAPPED,
902+
"BUG: settrap didn't restore correct ZSIG_TRAPPED");
903+
if ((sigtrapped[sig] = st->flags) & ZSIG_FUNC)
904+
shfunctab->addnode(shfunctab, ((Shfunc)st->list)->node.nam,
905+
(Shfunc) st->list);
906+
}
907+
878908
/**/
879909
void
880910
endtrapscope(void)
@@ -911,24 +941,7 @@ endtrapscope(void)
911941
remnode(savetraps, ln);
912942

913943
if (st->flags && (st->list != NULL)) {
914-
/* prevent settrap from saving this */
915-
dontsavetrap++;
916-
if (st->flags & ZSIG_FUNC)
917-
settrap(sig, NULL, ZSIG_FUNC);
918-
else
919-
settrap(sig, (Eprog) st->list, 0);
920-
if (sig == SIGEXIT)
921-
exit_trap_posix = st->posix;
922-
dontsavetrap--;
923-
/*
924-
* counting of nsigtrapped should presumably be handled
925-
* in settrap...
926-
*/
927-
DPUTS((sigtrapped[sig] ^ st->flags) & ZSIG_TRAPPED,
928-
"BUG: settrap didn't restore correct ZSIG_TRAPPED");
929-
if ((sigtrapped[sig] = st->flags) & ZSIG_FUNC)
930-
shfunctab->addnode(shfunctab, ((Shfunc)st->list)->node.nam,
931-
(Shfunc) st->list);
944+
restore_saved_trap(st);
932945
} else if (sigtrapped[sig]) {
933946
/*
934947
* Don't restore the old state if someone has set a
@@ -947,7 +960,12 @@ endtrapscope(void)
947960
* We already made sure this wasn't set as a POSIX exit trap.
948961
* We respect the user's intention when the trap in question
949962
* was set.
963+
*
964+
* If we are exiting, clear errflag so the trap can run.
965+
* Matches what zexit() does before calling dotrap().
950966
*/
967+
if (exit_pending)
968+
errflag = 0;
951969
dotrapargs(SIGEXIT, &exittr, exitfn);
952970
if (exittr & ZSIG_FUNC)
953971
shfunctab->freenode((HashNode)exitfn);
@@ -958,7 +976,6 @@ endtrapscope(void)
958976
"BUG: still saved traps outside all function scope");
959977
}
960978

961-
962979
/*
963980
* Decide whether a trap needs handling.
964981
* If so, see if the trap should be run now or queued.

Src/subst.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3343,15 +3343,23 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
33433343
*/
33443344
errflag |= ERRFLAG_HARD;
33453345
if (!interact) {
3346-
if (mypid == getpid()) {
3346+
if (locallevel > forklevel) {
33473347
/*
3348-
* paranoia: don't check for jobs, but there
3349-
* shouldn't be any if not interactive.
3348+
* Inside a function: use the same unwind
3349+
* mechanism as the exit builtin so that
3350+
* endtrapscope() runs at each level and
3351+
* all saved EXIT traps are executed.
3352+
*/
3353+
set_exit_pending(1);
3354+
} else {
3355+
/*
3356+
* Not inside a function: zexit() runs the
3357+
* EXIT trap and handles both main process
3358+
* and subshell (_exit) cases.
33503359
*/
33513360
stopmsg = 1;
33523361
zexit(1, ZEXIT_NORMAL);
3353-
} else
3354-
_exit(1);
3362+
}
33553363
}
33563364
}
33573365
return NULL;

Test/C03traps.ztst

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,47 @@ F:Must be tested with a top-level script rather than source or function
10141014
>one
10151015
>one
10161016

1017+
(setopt err_exit; trap 'echo EXIT' EXIT; f() { false; }; f)
1018+
1:EXIT trap called when ERR_EXIT triggers inside function
1019+
>EXIT
1020+
1021+
(setopt err_exit
1022+
trap 'echo A' EXIT
1023+
f1() {
1024+
trap 'echo B' EXIT
1025+
f2() {
1026+
trap 'echo C' EXIT
1027+
false
1028+
}
1029+
f2
1030+
}
1031+
f1)
1032+
1:nested EXIT traps all called when ERR_EXIT triggers inside function
1033+
>C
1034+
>B
1035+
>A
1036+
1037+
(trap 'echo EXIT' EXIT; f() { echo "a${unsetvar:?oops}z"; }; f)
1038+
1:EXIT trap called when ${var:?} triggers inside function
1039+
*?*unsetvar: oops
1040+
>EXIT
1041+
1042+
(trap 'echo A' EXIT
1043+
f1() {
1044+
trap 'echo B' EXIT
1045+
f2() {
1046+
trap 'echo C' EXIT
1047+
echo "a${unsetvar:?oops}z"
1048+
}
1049+
f2
1050+
}
1051+
f1)
1052+
1:nested EXIT traps all called when ${var:?} triggers inside function
1053+
*?*unsetvar: oops
1054+
>C
1055+
>B
1056+
>A
1057+
10171058
( set -o ERR_RETURN; f() { false; echo NOT REACHED; }; f || true; echo OK )
10181059
( set -o ERR_RETURN; f() { true && false; echo NOT REACHED; }; f || true; echo OK )
10191060
( set -o ERR_RETURN; f() { true && { false }; echo NOT REACHED; }; f || true; echo OK )

0 commit comments

Comments
 (0)