Skip to content

Commit

Permalink
Fix race condition scheduling $tempdir deletion
Browse files Browse the repository at this point in the history
The `cargo-zng` script contained:

    trap 'rm -rf "$tempdir"' 0 INT
    tempdir="$(mktemp -d)"

That releases a resource that may occasionally not have been
acquired.

In the rare case that the script would be interrupted before
`$tempdir` was set, this would attempt to read the previous value
of `$tempdir`. But if the calling environment had an environment
variable of this name, then its value would have been inherited as
the initial `$tempdir` in the script, and wrongly deleted.

The change made here puts the commands in the other order, so we
only arrange for the deletion of `$tempdir` once the directory we
want to delete exists and `$tempdir` will definitely expand to its
name and not that of another directory.

In a sense, this trades one race condition for another, in that
now the directory will not be deleted if the script is interrupted
immediately after it is created. But this is not a problem, because
it does not cause data loss, and the script already does not try to
cover all ways the script can be terminated in the `trap` command.
  • Loading branch information
EliahKagan committed Sep 6, 2024
1 parent bb9e89f commit 8bc6881
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion cargo-zng
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash
set -eu
trap 'rm -rf "$tempdir"' 0 INT
tempdir="$(mktemp -d)"
trap 'rm -rf "$tempdir"' 0 INT
cargo package -l --allow-dirty |
tr '\\' '/' |
grep -v '^Cargo\.toml\.orig$' |
Expand Down

0 comments on commit 8bc6881

Please sign in to comment.