fix(wake): #927 enqueue TOCTOU — move stale-tmp cleanup off the hot enqueue path (no concurrent in-flight-write clobber) (#928)
Co-authored-by: jason.woltje <jason@diversecanvas.com> Co-committed-by: jason.woltje <jason@diversecanvas.com>
This commit was merged in pull request #928.
This commit is contained in:
@@ -110,12 +110,28 @@ _wake_lock_release() {
|
||||
{ exec 8>&-; } 2>/dev/null || true
|
||||
}
|
||||
|
||||
# _wake_clean_stale_tmp DIR — remove leftover atomic-write temp files (e.g. from
|
||||
# a crash mid-write). Safe: these are never the live store.
|
||||
# _wake_clean_stale_tmp DIR — reap ORPHANED atomic-write temp files left by a
|
||||
# crash mid-write (a crash before the rename leaves a .wake.tmp.* that no reader
|
||||
# ever promotes). These are never the live store.
|
||||
#
|
||||
# AGE-SCOPED (#927): only tmp files whose mtime is older than
|
||||
# ${WAKE_TMP_STALE_MIN:-5} minutes are removed. A LIVE in-flight atomic write's
|
||||
# tmp is at most milliseconds old (mktemp -> cat -> sync -> rename all complete
|
||||
# well under a second), so it can NEVER match this age filter. That is what makes
|
||||
# the cleanup safe even if it ever overlaps a concurrent enqueue's atomic write:
|
||||
# it deletes only DEMONSTRABLY-orphaned tmps, never another process's live write.
|
||||
#
|
||||
# This is why #927 is fixed: an UNCONDITIONAL delete of every .wake.tmp.* (the
|
||||
# old behaviour) clobbered a concurrent enqueue's in-flight tmp -> spurious
|
||||
# "durable pending write FAILED". It is ALSO no longer invoked from the per-
|
||||
# enqueue hot path (see _wake_init_dir); it runs only at maintenance / daemon-
|
||||
# start (store.sh init) and the detector poll tick, where accumulation is bounded
|
||||
# once per pass rather than raced on every enqueue.
|
||||
_wake_clean_stale_tmp() {
|
||||
local dir="$1"
|
||||
local dir="$1" min="${WAKE_TMP_STALE_MIN:-5}"
|
||||
[ -d "$dir" ] || return 0
|
||||
find "$dir" -maxdepth 1 -name "${_wake_tmp_prefix}*" -type f -delete 2>/dev/null || true
|
||||
case "$min" in '' | *[!0-9]*) min=5 ;; esac
|
||||
find "$dir" -maxdepth 1 -name "${_wake_tmp_prefix}*" -type f -mmin "+$min" -delete 2>/dev/null || true
|
||||
}
|
||||
|
||||
# _wake_read_int FILE DEFAULT — read a single integer from FILE, or DEFAULT.
|
||||
@@ -133,10 +149,17 @@ _wake_read_int() {
|
||||
}
|
||||
|
||||
# _wake_init_dir STATE_DIR — ensure the state layout exists; idempotent.
|
||||
#
|
||||
# #927: this runs on the HOT enqueue path (cmd_enqueue calls it BEFORE taking the
|
||||
# enqueue lock) as well as on consume/cursors/ack. It must therefore NEVER touch
|
||||
# another process's tmp files: the old _wake_clean_stale_tmp call here deleted a
|
||||
# concurrent enqueue's LIVE in-flight tmp mid-write -> spurious durable-write
|
||||
# abort. Stale-tmp reaping is now an explicit maintenance action (store.sh init /
|
||||
# detector poll tick), NOT a side effect of ensuring the layout. Keep this
|
||||
# function limited to creating the dir + seeding the cursor files.
|
||||
_wake_init_dir() {
|
||||
local dir="$1"
|
||||
mkdir -p "$dir" || return 1
|
||||
_wake_clean_stale_tmp "$dir"
|
||||
[ -f "$dir/observed_seq" ] || printf '0' | _atomic_write "$dir/observed_seq"
|
||||
[ -f "$dir/consumed_seq" ] || printf '0' | _atomic_write "$dir/consumed_seq"
|
||||
[ -f "$dir/observed.set" ] || printf '' | _atomic_write "$dir/observed.set"
|
||||
|
||||
Reference in New Issue
Block a user