fix(update): always remove temp dir on exit#15
Open
CheapFuck wants to merge 1 commit into
Open
Conversation
The --apply path installed two separate EXIT traps: one to remove the download temp dir and a later one to roll back the backup. Bash keeps only the most recently registered trap, so the second registration silently dropped the temp-dir cleanup, and the final 'trap - EXIT' cleared it entirely. As a result the mktemp -d directory was leaked in /tmp after every successful update (and after failures past the backup step). Use a single EXIT trap with a cleanup function that always removes the temp dir and performs the backup rollback only when a restore_backup flag is set. The flag is raised before installing and cleared on success, so the rollback still works while the temp dir is always cleaned up. Fixes codam-coding-college#12
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
1 participant
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.
Summary
towel update --applyleaks themktemp -ddownload directory in/tmpafter every successful update.Root cause
The
--applypath registers two separateEXITtraps:trap '/usr/bin/rm -rf "$tmpdir"' EXIT— temp-dir cleanuptrap 'rm -rf "$TOWEL_DATA"; mv "$backup_current" ...' EXIT— backup rollbackBash keeps only the most recently registered
EXITtrap, so (2) silently replaces (1). On success the finaltrap - EXITthen clears the trap entirely, so$tmpdiris never removed (and it also leaks on any failure that happens after the backup step).Fix
Use a single
EXITtrap with a cleanup function that always removes$tmpdirand performs the backup rollback only when arestore_backupflag is set. The flag is raised right before installing and cleared on success, so rollback still works on failure while the temp dir is always cleaned up.Testing
shellcheck -xacross all scripts: passshfmt -dacross all scripts: passFixes #12