Fix EXIT trap not called when shell is exiting under numerous circumstances#164
Draft
tmoschou wants to merge 1 commit intozsh-users:masterfrom
Draft
Fix EXIT trap not called when shell is exiting under numerous circumstances#164tmoschou wants to merge 1 commit intozsh-users:masterfrom
tmoschou wants to merge 1 commit intozsh-users:masterfrom
Conversation
359b62d to
dc7a405
Compare
dc7a405 to
32e463f
Compare
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).
In endtrapscope(), convert errflag-driven exits to exit_pending
in non-interactive shells so that NOUNSET and other zerr()-based
errors also unwind through endtrapscope() properly.
32e463f to
c9055c9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
EXIT traps are never called when the shell is exiting under numerous circumstances.
ERR_EXIT(set -e) set and a command fails inside a functionzsh -e -c 'trap "echo Trap" EXIT; f() { false; }; f'${unsetvar?message}/${unsetvar:?message}used inside a function in a non-interactive shellzsh -c 'trap "echo Trap" EXIT; f() { echo ${unset?oops}; }; f'NO_UNSET(set -u) set and an unset variable is referenced (inside a function or not)zsh -u -c 'trap "echo Trap" EXIT; echo $unset'In all cases, expected:
Trapon stdout. Actual:Trapmissing.This contradicts the documentation and user expectation that EXIT can be relied on for clean-up (e.g. lock files, temporary files, etc.) when the shell is exiting. Additionally bash handles these cases as you would expect, always firing the EXIT trap.
Zsh also fails to fire nested local-scoped EXIT traps in these cases. However swapping
falseforexit 1does cause all EXIT traps to fire correctly bottom-up:Note that EXIT traps specifically are local by default (
LOCA_TRAPSbehavior), unlessPOSIX_TRAPSis set.Root cause
When a function is entered,
starttrapscope()saves and unsetssigtrapped[SIGEXIT]. Normallyendtrapscope()restores it when the function returns.ERR_EXIT: When
ERR_EXITtriggers inside a function,execlist()calledrealexit()directly — bypassingendtrapscope()— so saved EXIT traps were never restored or executed.${var:?}: Theparamsubst()handler called_exit(1)(subshell) orzexit()(main process) directly, similarly bypassing the function unwind andendtrapscope(). Additionally,errflag(set byzerr()) preventeddotrapargs()from executing the EXIT trap even when the unwind did reachendtrapscope().NO_UNSET:paramsubst()andgetmathparam()seterrflagviazerr()and returned, buterrflagblockeddotrapargs()inendtrapscope()from running EXIT traps at each function level. The shell eventually exited via the main loop without unwinding through function scopes.Fix
Reuse the same
exit_pendingunwind mechanism as theexitbuiltin, so thatendtrapscope()runs at each function level and all nested EXIT traps are executed bottom-up, matching the behaviour of an explicitexitcall.${var:?}: callset_exit_pending()instead ofrealexit()/zexit()when inside a function, so the exit unwinds through each scope.NO_UNSETand otherzerr()-based errors: inendtrapscope(), converterrflag-driven exits toexit_pendingin non-interactive shells, so all EXIT traps fire during the unwind. This is a single central check rather than patching each error site.set_exit_pending()to consolidate the repeated unwind setup (trap_state,retflag,breaks,exit_pending,exit_level,exit_val).restore_saved_trap()fromendtrapscope()to reduce duplication in the trap restoration logic.errflaginendtrapscope()whenexit_pending, so EXIT traps can fire even when the exit was triggered by an error. This mirrors whatzexit()already does before callingdotrap(SIGEXIT).Test plan
Test/C03traps.ztstforERR_EXIT,${var:?}, andNO_UNSET— both top-level and nested function casesTrap:POSIX_TRAPSstill behaves correctly (EXIT traps are global, not function-scoped):./Src/zsh -f -o posix_traps -e -c 'trap "echo A" EXIT; f() { trap "echo B" EXIT; }; f; false'