Skip to content

Repository navigation

fix(hostlock): a host that cannot take the lock must say UNUSABLE, not FREE #1989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
justinchuby merged 2 commits into from
Aug 24, 2026
Merged

fix(hostlock): a host that cannot take the lock must say UNUSABLE, not FREE #1989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1338bdf
fix(hostlock): a host that cannot take the lock must say UNUSABLE, no…
Aug 24, 2026
cd9e7d0
test(hostlock): pin the preflight ahead of the legacy consult
Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 116 additions & 5 deletions scripts/hostlock.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@
# wrapped command that exits 6 by itself, exactly as 5 collides with the
# gate: `run` multiplexes two status spaces onto one, and the `cpu ...
# verdict=` line, not the exit code, is what tells them apart.
# 7 lock dir unusable -- no lock can be created at LOCK_DIR on this host, so
# the answer is neither "yours" nor "somebody else's". It is separate from
# 1 because a misconfigured or unwritable host is not a bad argument and
# must not be retried as one, and separate from 2 and 3 because those two
# assert a peer holds the box. See lock_dir_problem.
#
# `run` otherwise does NOT use this table: it returns the wrapped command's
# own status, so a command exiting 5 is indistinguishable from a gate failure.
Expand Down Expand Up @@ -922,8 +927,75 @@ publish_lock() {
# reads STALE by the liveness test and BUSY by `acquire`, and of those two
# the reassuring one is STALE: it tells a human the box is abandoned, and
# they take it. Report what the tool will actually do.
# Echo why no lock can be taken at LOCK_DIR and return 0; return 1 when one can.
#
# FREE is not a fact about the directory, it is a promise to the caller: "the
# host is yours, go ahead". On a host where the lock cannot be CREATED that
# promise is false, and until this existed `status` made it anyway -- printing
# `FREE (runnable=4)`, and `state=FREE` in porcelain, in bytes IDENTICAL to a
# genuinely free and usable host. That is the same defect `warn_if_private`
# was written for one level up (#1942): output that is not wrong about what it
# measured, and silent about what it measured.
#
# It is not hypothetical here. This box's default lock_dir is under /tmp, and
# at least one agent on it is under a hard prohibition on writing /tmp at all;
# `acquire` failed for that agent with a raw `mkdir: Permission denied` and
# exit 1 -- "usage/error", the code for a bad argument -- while `status` went
# on saying FREE. An unattended harness reading either one proceeds unlocked,
# which is precisely what the lock became mandatory to prevent.
#
# What is tested is the PARENT, never LOCK_DIR itself: publish_lock stages at
# `${LOCK_DIR}.stage.$$`, a SIBLING, and renames it into place. A lock dir that
# exists but is unwritable is therefore still perfectly publishable, and an
# absent one whose parent is writable is fine too -- so the question is always
# "can we create entries next to it", answered at the nearest ancestor that
# exists, because `mkdir -p` will make the rest.
#
# `-w` lies under CAP_DAC_OVERRIDE, so root sees "usable" where a mortal would
# not. That is the safe direction: it is exactly today's behaviour, and root
# can in fact write there.
lock_dir_problem() {
local p
if [ -e "$LOCK_DIR" ] && [ ! -d "$LOCK_DIR" ]; then
printf '%s\n' "${LOCK_DIR} exists and is not a directory"
return 0
fi
case "$LOCK_DIR" in
*/*) p=${LOCK_DIR%/*}; [ -n "$p" ] || p=/ ;;
*) p=. ;;
esac
while [ ! -e "$p" ]; do
case "$p" in
*/*) p=${p%/*}; [ -n "$p" ] || p=/ ;;
*) p=. ; break ;;
esac
done
if [ ! -d "$p" ]; then
printf '%s\n' "${p} exists and is not a directory"
return 0
fi
if [ ! -w "$p" ] || [ ! -x "$p" ]; then
printf '%s\n' "${p} is not writable by $(id -un 2>/dev/null || echo "uid $(id -u 2>/dev/null)")"
return 0
fi
return 1
}

# Say the UNUSABLE part out loud, on stderr, wherever a caller was about to be
# told the host is available. Three lines, because one is not enough to stop an
# agent that has been told the lock is mandatory and has just read "free".
explain_unusable() {
local problem=$1
echo "hostlock: UNUSABLE: ${problem}" >&2
echo "hostlock: no lock can be created at ${LOCK_DIR} (${LOCK_DIR_SOURCE}, ${LOCK_SCOPE}), so this host cannot participate." >&2
echo "hostlock: set lock_dir in ${HOSTLOCK_CONF_PATH} to a path you can write; do NOT run saturating benchmarks unlocked." >&2
}

lock_state() {
[ -d "$LOCK_DIR" ] || { echo FREE; return 0; }
if [ ! -d "$LOCK_DIR" ]; then
if lock_dir_problem >/dev/null; then echo UNUSABLE; else echo FREE; fi
return 0
fi
# unverifiable_live_anchor is the class this tool cannot verify but can
# SEE running. Routing it through the STALE arm made `status` say "is
# gone" about a pid the reaper had just confirmed present -- the same
Expand Down Expand Up @@ -1100,6 +1172,22 @@ status_lock_dir_note() {
return 0
}

print_free_or_unusable() {
local problem
if problem=$(lock_dir_problem); then
echo "UNUSABLE ${problem}"
# Exactly one lock line either way: status_lock_dir_note prints it for
# every non-default source, and the default source is the case that
# needs it most -- /tmp is where this actually bites.
[ "$LOCK_DIR_SOURCE" = default ] && echo " lock: ${LOCK_DIR} (${LOCK_DIR_SOURCE}, ${LOCK_SCOPE})"
echo " no lock can be created here; this host cannot participate"
echo " runnable=$(runnable_now)"
return 0
fi
echo "FREE (runnable=$(runnable_now))"
return 0
}

cmd_status() {
local legacy
legacy=$(legacy_holder) || legacy="none"
Expand All @@ -1109,6 +1197,11 @@ cmd_status() {
echo "lock_dir=${LOCK_DIR}"
echo "lock_scope=${LOCK_SCOPE}"
echo "lock_dir_source=${LOCK_DIR_SOURCE}"
# Always emitted, empty when there is none, so the key set does not
# change shape between hosts -- a consumer that has to test for a key's
# PRESENCE to learn the state is one `grep` away from reading absence
# as "fine", which is the failure this whole field exists to report.
echo "lock_dir_problem=$(lock_dir_problem || true)"
echo "legacy_dir=$(legacy_consult_path)"
echo "legacy_held_by=${legacy%% *}"
if [ -d "$LOCK_DIR" ]; then
Expand All @@ -1121,7 +1214,7 @@ cmd_status() {
return 0
fi
if [ ! -d "$LOCK_DIR" ]; then
echo "FREE (runnable=$(runnable_now))"
print_free_or_unusable
status_lock_dir_note "$legacy"
return 0
fi
Expand Down Expand Up @@ -1157,7 +1250,7 @@ cmd_status() {
*)
# lock_state saw no lock dir; it was released under us since the
# test above. Report what is true now rather than a stale label.
echo "FREE (runnable=$(runnable_now))"
print_free_or_unusable
;;
esac
status_lock_dir_note "$legacy"
Expand All @@ -1178,7 +1271,18 @@ abandon_lock() {
}

cmd_acquire() {
local deadline=$((SECONDS + TIMEOUT)) rc
local deadline=$((SECONDS + TIMEOUT)) rc problem
# Refuse BEFORE anything else, including the legacy consult. An acquire
# that cannot possibly publish must not spend --timeout looking busy: with
# --wait (which `run` always sets) the unusable host reported "timed out
# after 900s waiting for the lock", i.e. it blamed peers for contention
# that did not exist, and returned 3 -- a code whose documented meaning is
# that somebody else has the box. Distinct outcomes, or the label launders
# the fault.
if problem=$(lock_dir_problem); then
explain_unusable "$problem"
return 7
fi
refuse_if_legacy_held || return $?
while :; do
if publish_lock; then
Expand Down Expand Up @@ -1325,7 +1429,14 @@ cmd_release() {
}

cmd_wait() {
local deadline=$((SECONDS + TIMEOUT))
local deadline=$((SECONDS + TIMEOUT)) problem
# An unusable lock dir is never HELD, so the loop below falls straight
# through and announces "free" -- the one word this caller is waiting to
# hear, on the one host where it cannot be true.
if problem=$(lock_dir_problem); then
explain_unusable "$problem"
return 7
fi
# Wait only while somebody actually has a live claim. An EXPIRED lock is
# one the next acquirer would take over, so blocking on it would disagree
# with what `status` and `acquire` both say.
Expand Down
175 changes: 173 additions & 2 deletions scripts/hostlock_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ HL=scripts/hostlock.sh
pass=0
fail=0

cleanup() { rm -rf "$LOCK" "$LOCK".cpuself "$LOCK".reaper "$LOCK".reaper.stage.* "$LOCK".reaper.dead.* "$LOCK".reaper.rel.* "$LOCK".dead.* "$LOCK".stage.* "$LOCK".gate "$LOCK".warn "$LOCK".zombie.* "$LOCK".zpid "$LOCK".ttlmarker "$LOCK".ran "$LOCK".conf "$LOCK".legacy "$LOCK".box "$LOCK".sourced "$LOCK".legacyran "$LOCK".owner "$LOCK".nested 2>/dev/null; }
cleanup() { rm -rf "$LOCK" "$LOCK".cpuself "$LOCK".reaper "$LOCK".reaper.stage.* "$LOCK".reaper.dead.* "$LOCK".reaper.rel.* "$LOCK".dead.* "$LOCK".stage.* "$LOCK".gate "$LOCK".warn "$LOCK".zombie.* "$LOCK".zpid "$LOCK".ttlmarker "$LOCK".ran "$LOCK".conf "$LOCK".legacy "$LOCK".box "$LOCK".sourced "$LOCK".legacyran "$LOCK".owner "$LOCK".nested "$LOCK".ro "$LOCK".deep "$LOCK".file "$LOCK".nox "$LOCK".conf2 2>/dev/null; }
trap cleanup EXIT

chk() {
Expand Down Expand Up @@ -2070,13 +2070,184 @@ chk "and that place is cmd_run, which is the only one with a child" \
rm -rf "$LOCK.nested"
cleanup

# == a host that cannot take the lock must say so, not say FREE ==
#
# FREE is read as "the host is yours, go ahead". Where the lock directory
# cannot be created that reading is false, and the tool used to make it in
# three measured ways. All six lines below are what `origin/main` does, run
# against these same two fixtures rather than reasoned about:
#
# unwritable parent (chmod 500):
# status FREE (runnable=4) rc 0 <-- fail open
# status --porcelain state=FREE rc 0 <-- fail open
# wait hostlock: free rc 0 <-- fail open
# acquire / run mkdir: ... Permission denied rc 1 <-- refuses, but
# as "usage/error"
# lock path is a plain file:
# acquire --wait timed out after 12s waiting for the lock, AND
# prints FREE in the same breath rc 3
#
# The first three tell a harness to proceed on the one host where the lock is
# unavailable -- `status` is the README's own "is anybody benchmarking?" and
# `wait` exists to be followed by a run. The fourth is a correct refusal with
# a wrong classification: exit 1 is the code for a bad argument, so an
# unattended caller cannot tell a broken host from its own typo. The fifth is
# worse than either, because the box is idle and the tool spends the whole
# --timeout before blaming peers for contention that does not exist -- while
# printing FREE underneath it.
#
# This matters beyond the fixtures: the lock is mandatory for saturating runs,
# and the default lock_dir is under /tmp, which not every host can use.
UNUSABLE_PARENT="$LOCK.ro"
rm -rf "$UNUSABLE_PARENT"
mkdir -p "$UNUSABLE_PARENT"
chmod 500 "$UNUSABLE_PARENT"
UNUSABLE_DIR="$UNUSABLE_PARENT/hl"

chk "status calls an uncreatable lock dir UNUSABLE, not FREE" \
"$(HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" status 2>/dev/null | head -1 | awk '{print $1}')" \
"UNUSABLE"
chk "and porcelain says so in the state field" \
"$(HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" status --porcelain 2>/dev/null | sed -n 's/^state=//p')" \
"UNUSABLE"
chk "porcelain names the reason rather than only the verdict" \
"$(HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" status --porcelain 2>/dev/null | sed -n 's/^lock_dir_problem=//p' | grep -c "$UNUSABLE_PARENT")" \
"1"
# The negative control for the three above, and the reason the key is emitted
# unconditionally: a consumer that learns the state from the key's PRESENCE
# reads absence as "fine", which is the failure the field exists to report.
chk "a usable free host emits the same key, empty" \
"$(HOSTLOCK_DIR="$LOCK" "$HL" status --porcelain 2>/dev/null | grep -c '^lock_dir_problem=$')" \
"1"
chk "and still calls itself FREE" \
"$(HOSTLOCK_DIR="$LOCK" "$HL" status 2>/dev/null | head -1 | awk '{print $1}')" \
"FREE"

# Both halves of the guard, separately. A directory that is writable but not
# SEARCHABLE cannot host entries either -- `mkdir 0600-parent/sub` fails with
# EACCES -- and every other fixture in this section is chmod 500, i.e. not
# writable but executable, which exercises only the `-w` clause. Without this
# cell, deleting `|| [ ! -x "$p" ]` is a one-line fail-open that the whole
# suite stays green through.
NOSEARCH_PARENT="$LOCK.nox"
rm -rf "$NOSEARCH_PARENT"
mkdir -p "$NOSEARCH_PARENT"
chmod 600 "$NOSEARCH_PARENT"
chk "a writable but unsearchable parent is UNUSABLE too" \
"$(HOSTLOCK_DIR="$NOSEARCH_PARENT/hl" "$HL" status --porcelain 2>/dev/null | sed -n 's/^state=//p')" \
"UNUSABLE"
chmod 700 "$NOSEARCH_PARENT"
rm -rf "$NOSEARCH_PARENT"

HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" acquire --owner leon >/dev/null 2>&1
chk "acquire exits 7, not 1 (a bad host is not a bad argument)" "$?" "7"
HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" wait >/dev/null 2>&1
chk "wait exits 7 instead of announcing free" "$?" "7"
chk "and prints no free announcement at all" \
"$(HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" wait 2>/dev/null | grep -c free)" \
"0"
chk "the refusal names the path, so it is actionable" \
"$(HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" acquire --owner leon 2>&1 >/dev/null | grep -c "$UNUSABLE_DIR")" \
"1"

rm -f "$LOCK.ran"
HOSTLOCK_DIR="$UNUSABLE_DIR" "$HL" run --owner leon -- touch "$LOCK.ran" >/dev/null 2>&1
chk "run exits 7" "$?" "7"
chk "and does not run the command" \
"$([ -e "$LOCK.ran" ] && echo ran || echo refused)" "refused"
rm -f "$LOCK.ran"

# A path that exists as a FILE can never become a lock dir, and this is the
# fixture where the pre-fix loop is reachable: the staging dir is a SIBLING,
# so it is created fine, and only the final `mv -T` onto the file fails --
# which publish_lock reports the same way it reports "somebody got there
# first". So the old code looped, slept, and after --timeout announced
# contention on an idle box. Timed as well as coded, because a refusal that
# takes --timeout to arrive is still the defect: `run` sets --wait
# unconditionally, so this did not fail, it HUNG.
: >"$LOCK.file"
chk "a lock path that is a plain file is UNUSABLE" \
"$(HOSTLOCK_DIR="$LOCK.file" "$HL" status --porcelain 2>/dev/null | sed -n 's/^state=//p')" \
"UNUSABLE"
t0=$SECONDS
HOSTLOCK_DIR="$LOCK.file" "$HL" acquire --wait --timeout 30 --owner leon >/dev/null 2>&1
rc=$?
chk "acquire --wait refuses instead of waiting out the timeout" "$rc" "7"
chk "and refuses promptly rather than blaming peers for 30s" \
"$([ "$((SECONDS - t0))" -lt 5 ] && echo prompt || echo slow)" "prompt"
rm -f "$LOCK.file"

# The check is on the PARENT, never the leaf. publish_lock stages at a SIBLING
# of LOCK_DIR and renames it into place, so "can we create entries next to it"
# is the real question -- and `mkdir -p` will build any missing intermediate
# levels, so a deep path under a writable ancestor is usable too. A guard that
# demanded the immediate parent exist would refuse this host wrongly, which is
# the mirror-image failure: a lock that refuses a usable box is a lock nobody
# keeps using.
DEEP="$LOCK.deep/a/b/hl"
rm -rf "$LOCK.deep"
chk "a deep path under a writable ancestor is usable" \
"$(HOSTLOCK_DIR="$DEEP" "$HL" status --porcelain 2>/dev/null | sed -n 's/^state=//p')" \
"FREE"
HOSTLOCK_DIR="$DEEP" "$HL" run --owner leon -- true >/dev/null 2>&1
chk "and can actually be acquired, mkdir -p building the rest" "$?" "0"
rm -rf "$LOCK.deep"

# Usability must not leak into the occupied path: a HELD lock is HELD whatever
# the directory's mode says, or the state that stops a second agent taking the
# box would be answerable by a chmod.
"$HL" acquire --owner leon-held --ttl 60 >/dev/null 2>&1
chk "a held lock still reads HELD, not UNUSABLE" \
"$("$HL" status --porcelain 2>/dev/null | sed -n 's/^state=//p')" "HELD"

# The sharp end of "the parent, never the leaf", and the cell that a mutant
# testing LOCK_DIR itself survives without: a lock dir that EXISTS and is
# unwritable is still perfectly publishable (the stage is a sibling), and the
# honest answer for a second agent is BUSY -- somebody has the box. Reporting
# UNUSABLE there would relabel real contention as a broken host, which is the
# most expensive direction to get this wrong: it tells the agent to go fix its
# config and try again, while a peer's benchmark is running.
chmod 500 "$LOCK"
"$HL" acquire --owner leon-other >/dev/null 2>&1
chk "a held-but-unwritable lock dir is BUSY, not UNUSABLE" "$?" "2"
chmod 700 "$LOCK"
"$HL" release >/dev/null 2>&1
rm -rf "$LOCK"

# Ordering pin. The preflight has to run BEFORE the legacy consult, and that
# ordering is invisible to every cell above: they all set HOSTLOCK_DIR, so
# LOCK_DIR_SOURCE is `env`, and refuse_if_legacy_held returns immediately for
# any source but `config`. Swap the two and the whole suite stays green while
# a host that cannot take the lock at all is told a peer has the box -- exit 2
# instead of 7, which sends the agent away to wait for a lock it could never
# have acquired. Raised by review as the one surviving mutation; this is the
# fixture that reddens it.
rm -rf "$LOCK.legacy"
printf 'lock_dir=%s\n' "$UNUSABLE_DIR" >"$LOCK.conf2"
sleep 300 &
unusable_legacy_pid=$!
unusable_legacy_start=$(sed 's/.*) //' "/proc/${unusable_legacy_pid}/stat" | awk '{print $20}')
mkdir -p "$LOCK.legacy"
printf 'owner=roy\nanchor_pid=%s\nstart_time=%s\nacquired_epoch=%s\nttl=0\nreason=prefill matrix\n' \
"$unusable_legacy_pid" "$unusable_legacy_start" "$(date +%s)" >"$LOCK.legacy/meta"
chk "an unusable configured dir refuses as 7 even while the legacy path is live" \
"$(env -u HOSTLOCK_DIR HOSTLOCK_CONF="$LOCK.conf2" HOSTLOCK_LEGACY_DIR="$LOCK.legacy" \
"$HL" acquire --owner leon >/dev/null 2>&1; echo $?)" "7"
kill "$unusable_legacy_pid" 2>/dev/null
wait "$unusable_legacy_pid" 2>/dev/null
rm -rf "$LOCK.legacy" "$LOCK.conf2"

chmod 700 "$UNUSABLE_PARENT" 2>/dev/null
rm -rf "$UNUSABLE_PARENT"
cleanup

# Finally, pin the assertion count itself. Two of the checks in this file sit
# behind environment probes, and an assertion that quietly stops running is
# indistinguishable from one that passes -- which is the same failure mode as
# the inert R1 block and the vacuous STALE arm that this PR exists to fix.
# Both probe branches now assert something, so the total is invariant across
# environments; if a refactor drops a check, this fails and says so.
chk "every assertion in this file ran" "$((pass + fail + 1))" "321"
chk "every assertion in this file ran" "$((pass + fail + 1))" "341"

echo
echo "passed=${pass} failed=${fail}"
Expand Down
Loading
Loading