diff options
author | Lukas Fleischer <archlinux@cryptocrack.de> | 2011-10-13 17:23:19 +0200 |
---|---|---|
committer | Dan McGee <dan@archlinux.org> | 2011-10-13 18:20:03 +0200 |
commit | d4c97ea2f64cafd3e14e2817d2b805f0b0d541f1 (patch) | |
tree | 4639bea0a033e8351fb575053a25060740efbd05 /scripts | |
parent | 12642a299b9f4218f43ce8a4f1d9924cfae744ee (diff) | |
download | pacman-d4c97ea2f64cafd3e14e2817d2b805f0b0d541f1.tar.gz pacman-d4c97ea2f64cafd3e14e2817d2b805f0b0d541f1.tar.xz |
repo-add: Avoid race condition in signal handlers
There is a small chance that a user sends SIGINT (or any other signal
that is trapped) when we're already in clean_up() which used to lead to
trap_exit() being executed and the remaining code in clean_up() being
skipped due to the bash signal/trap handler blocking EXIT (since its
handler is already being executed, even if it's interrupted).
In practice, this behaviour caused unexpected results (primarily because
pressing ^C at the wrong time left a lock file behind):
$ ./repo-add extra.db.tar.gz foobar
==> Extracting database to a temporary location...
^C
==> ERROR: Aborted by user! Exiting...
$ ./repo-add extra.db.tar.gz foobar
==> Extracting database to a temporary location...
==> ERROR: File 'foobar' not found.
==> No packages modified, nothing to do.
^C
==> ERROR: Aborted by user! Exiting...
$ ./repo-add extra.db.tar.gz foobar
==> ERROR: Failed to acquire lockfile: extra.db.tar.gz.lck.
==> ERROR: Held by process 18522
Fix this and reduce the chance of race conditions in signal handlers by:
* Unhooking all traps in both clean_up() and trap_exit().
* Call clean_up() explicitly in trap_exit() to make sure we remove the
lock file and the temporary directory even if we send SIGINT when
clean_up() is already being executed but didn't reach the unhook code
yet.
Also, add an optional parameter to clean_up() to allow for setting an
explicit exit code when we call clean_up() from trap_exit().
Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de>
Signed-off-by: Dan McGee <dan@archlinux.org>
Diffstat (limited to 'scripts')
-rw-r--r-- | scripts/repo-add.sh.in | 10 |
1 files changed, 8 insertions, 2 deletions
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 29b150c8..d147b875 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -520,13 +520,19 @@ remove() { } trap_exit() { + # unhook all traps to avoid race conditions + trap '' EXIT TERM HUP QUIT INT ERR + echo error "$@" - exit 1 + clean_up 1 } clean_up() { - local exit_code=$? + local exit_code=${1:-$?} + + # unhook all traps to avoid race conditions + trap '' EXIT TERM HUP QUIT INT ERR [[ -d $tmpdir ]] && rm -rf "$tmpdir" (( CLEAN_LOCK )) && [[ -f $LOCKFILE ]] && rm -f "$LOCKFILE" |