feat(bench): record in every row whether the host was declared

[justinchuby]

Closes #1924.

scripts/hostlock.sh has been on main for a while and nothing reads it:

$ git grep -l hostlock -- crates/ scripts/ .github/
.github/workflows/hostlock.yml
scripts/hostlock.sh
scripts/hostlock_test.sh

Every consumer is the script or its own test. A capability with no caller is
indistinguishable in the output from one that was never built, so a row printed
on a host somebody else had declared looks exactly like a row printed on a quiet
one. This adds the reader and puts the answer in the row.

What lands

onnx_runtime_hostmon::hostlock — a read-only consumer — plus a host_lock=
field on bench_generic rows and a provenance line on the
decode_gap_park_ab matrix.

value	meaning
free	nobody declared the host for the whole window
mine:<owner>	held throughout by a live anchor whose owner matches HOSTLOCK_OWNER — the only value that certifies a row
foreign:<owner>	held throughout by somebody else
held:<owner>	held throughout, HOSTLOCK_OWNER unset, so the reader cannot attribute it
unverified:<owner>	held, liveness unprovable (no anchor PID, or no recorded start_time)
stale:<owner>	held by an anchor that is provably gone
changed	the two readings disagree; the row spans a change of custody
unknown	the lock could not be read — not the same as free

Five decisions, and why

Read-only, and it stays that way. It never acquires or releases. Taking a
lock as a side effect of formatting a field would be far worse than no lock at
all.

Read at both ends, not once. changed is the whole point. A single reading
after the runs would report one credible holder for a window that changed hands
halfway through — the same stale-snapshot error as checking ps once before
starting, only moved into the row where it is harder to notice. Demonstrated end
to end below.

Liveness mirrors the script rather than being decided again. hostlock.sh's
comment on anchor_alive records that the last time two call sites decided this
question independently the answers disagreed, and two of the four defects in
#1830 came out of the gap. A Rust reader deciding it independently would be a
third call site, disagreeing in the worst possible place: the published row.

Ignorance is never resolved in the flattering direction. free and
unknown format differently; held and mine format differently;
unverified and stale format differently. is_protected() is Mine(_)
alone — an unattributable lock, an unverifiable anchor and a dead one all
refuse to certify a row, even when the owner string matches.

Owners are sanitised. The script's own header documents this hazard against
its provenance line: an owner of gaff hostlock_state=FREE declared=no splices
two extra key/value pairs into the output, and a consumer reading the first
hostlock_state= gets FREE for a held lock. A result row is a key=value
list too, so it inherits the hazard verbatim. The test asserts the field count
of the rendered row, not a string.

The integration test found two real defects in this reader

tests/agrees_with_hostlock_sh.rs drives the actual script — acquires,
releases, and reads back — rather than parsing a fixture I wrote myself. The
unit tests are pure functions over strings of my own construction, so they prove
the decision table is internally consistent and prove nothing about whether it
describes the file hostlock.sh writes.

It failed on first run, twice, both times on this reader and not on the script:

1. A zombie has a /proc/<pid> entry and a matching start time, so my
   original Path::exists liveness check reported a held lock on a corpse
   forever. The script calls this the common shape rather than the exotic one,
   because every agent harness on this box launches long commands without an
   immediate wait(). The test reproduces a genuine zombie — a spawned child
   that is never reaped, with the Z state asserted before the lock is taken —
   so it also confirms proc_info parses a real /proc/<pid>/stat.
2. A recycled PID passes an existence check too. Fixed by carrying the
   recorded start_time, which is exactly why the script records it.

State Z is not itself proof of death — a thread-group leader that exited via
pthread_exit while its threads keep running reports Z for a fully live
process, and reaping that one takes a live holder's machine mid-benchmark. So
Threads: decides, and an unreadable Threads: is no evidence of death.

The test fails rather than skips if the script is missing. A skip would be
the same defect one level up: the run prints ok, the agreement is unchecked,
and the output is indistinguishable from a real pass. The platform gate is
#[cfg], so on non-Linux the tests do not exist rather than passing vacuously.

The field prints on the unmeasured path too

That is the row with the least other evidence about the conditions it was taken
under. Dropping the declaration exactly there would leave the least trustworthy
rows looking the least suspicious.

It stays advisory

decode_gap_park_ab prints an UNPROTECTED warning to stderr and then prints
the matrix anyway. Refusing to publish because nobody took a lock would mostly
teach people to stop taking the lock.

Validation

End-to-end, on a scratch HOSTLOCK_DIR, each cross-checked against
hostlock.sh status --porcelain:

configuration	bench_generic printed	script agreed
lock held by sebastian, HOSTLOCK_OWNER=sebastian	host_lock=mine:sebastian	state=HELD
same lock, HOSTLOCK_OWNER=roy	host_lock=foreign:sebastian	state=HELD
same lock, HOSTLOCK_OWNER unset	host_lock=held:sebastian	state=HELD
anchor killed, lock left behind	host_lock=stale:sebastian	state=STALE
released 2 s into a 5 s run	host_lock=changed	—
no lock	host_lock=free	state=FREE

The fifth row is the one that justifies the design: a single end-of-run read
would have printed mine:sebastian there, and looked entirely normal.

Also read against the real lock while a co-tenant held it, and against it
after they released — reader and script agreed both times.

Tests: 19 unit + 2 integration in hostmon, 39 in bench_generic (3 new).
cargo fmt --all --check clean; clippy -D warnings clean on all three
changed crates.

Mutation: 21 mutants, 21 killed — including zombie-ignored,
zombie-leader-reaped, start-time-ignored, no-two-ended-read, owner-passthrough,
io-error-is-free, and every widening of is_protected.

Not in scope

Holding the lock from inside a harness. That is a decision a harness makes and
this deliberately does not make it.

Coordination

This is the reader half of the mechanical-lock idea @roy raised. I claimed it
after finding the writer already existed on main; the widening of the
realized-width assertion in matmul_nbits.rs is his and is untouched here.

[codecov]

Codecov Report

❌ Patch coverage is 88.79310% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.35%. Comparing base (26974e5) to head (d02c760).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/onnx-runtime-hostmon/src/hostlock.rs	88.79%	23 Missing and 16 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1925      +/-   ##
==========================================
+ Coverage   80.20%   80.35%   +0.15%     
==========================================
  Files         399      416      +17     
  Lines      186061   205223   +19162     
  Branches   186061   205223   +19162     
==========================================
+ Hits       149231   164916   +15685     
- Misses      31470    34711    +3241     
- Partials     5360     5596     +236     

Flag	Coverage Δ
cli-ort-linux	72.51% <ø> (?)
cli-ort-windows	72.01% <ø> (?)
mlas	85.20% <ø> (?)
offline	80.49% <88.79%> (+0.28%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
crates/onnx-runtime-hostmon/src/lib.rs	85.44% <ø> (-0.71%)	⬇️
crates/onnx-runtime-hostmon/src/hostlock.rs	88.79% <88.79%> (ø)

... and 64 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[justinchuby]

Independent adversarial review — three real findings, all fixed in 1898114f8

Ran an independent Opus adversarial review against scripts/hostlock.sh as the
reference implementation. It found three defects and cleared the rest. All three
are fixed; none of them were in the liveness logic, which is the part I expected
to be wrong.

1. decode_gap_park_ab read the lock three times at the end

host_lock= came from read #2 and lock_reason= from read #3, so the two
fields could describe different snapshots. The concrete bad output is
host_lock=changed lock_reason=acc0 — a reason naming a single holder for a
window the field itself says had none, which invites a reader to dismiss the
changed. That is the one value in the whole scheme that exists to stop a row
being believed, so undermining it with an adjacent field is the worst place to
put this bug.

It could not flip the integrity verdict (is_protected() derives only from the
before/read-#2 pair), so this is a self-contradictory row rather than a
laundered one. One reading now feeds both. bench_generic already did it
correctly, which is how the inconsistency survived my own reading of the diff.

2. Attribution compared the sanitised owner — a foreign → mine path

This is the real find. is_protected() rested on
sanitise_owner(HOSTLOCK_OWNER) == holder.owner, and sanitising is lossy in
precisely the wrong direction:

• sebastian!, sebastian and _sebastian_ all sanitise to sebastian;
• any two names ≥33 characters sharing a 32-character prefix collide under
  MAX_OWNER_LEN truncation.

So another agent's declaration could be promoted to mine and mark a
contaminated row protected — the single direction this module exists to
prevent. I had a test asserting truncation bounds the length and no test
asserting it does not merge identities, which is the same instrument-vs-
property gap I have been logging in other people's code all week.

LockHolder now carries owner_raw (the owner as written, minus surrounding
whitespace) and attribution compares that. Display stays sanitised, because the
row-forging hazard is equally real. Display may be lossy; the protection
decision may not.

Worth being clear about what this does not buy: owner is unauthenticated by
design — the script's header says it "corroborates nothing", and all agents on
this box share a UID, so a hostile holder can always just write my exact name.
The fix removes an accidental collision, not an adversarial one. Re-smoked end
to end: with the lock held by sebastian, HOSTLOCK_OWNER=sebastian gives
mine:sebastian while sebastian! and roy both give foreign:sebastian.

3. The integration test leaked its anchor and scratch dir on a failing assert

Cleanup ran after the assertions, so any panic between spawning the sleep 300
anchor and killing it left it reparented to init for five minutes. Both are RAII
guards now. It burns no CPU so it would not have corrupted anyone's measurement
— but complaining about other agents' leaked processes while leaking one on
every failed assertion is not a position worth defending, and a failing run is
exactly when someone is looking at the process table.

Traced and unfounded

Recording these so they are not re-litigated:

• liveness() vs anchor_alive+pid_is_live. The full cross-product of
  {anchor present/absent} × {start_time present/absent/mismatched} × {state Z /
  non-Z} × {Threads: unreadable/1/>1} × {/proc entry present/absent} was
  traced. Every case the script proves dead maps to Stale; every case it
  proves alive maps to Held. The only divergences are the no-start_time
  cases, where the script says HELD and the reader says Unverified — strictly
  more conservative, never flattering. Held (the sole route to Mine) requires
  a live non-zombie anchor and a matching start_time, which is exactly
  anchor_alive.
• proc_info field indexing. rfind(')') matches the script's greedy
  sed 's/.*) //' and survives a comm containing spaces or parentheses;
  next() is state (proc(5) field 3) and nth(18) is starttime (field 22).
  Independently confirmed by the agreement test, which compares the reader's
  parsed start_time against the value the script wrote — an off-by-one would
  flip Held to Stale and fail it.
• before != after over the whole holder. No spurious changed: the
  script's meta_set only ever rewrites takeover/gate, and ProcInfo.state
  and threads are not stored in LockState, so R↔S jitter that does not cross
  the zombie boundary produces an identical value. The same-owner
  release-and-reacquire false negative is the inherent limit of two-point
  sampling, and is benign when the measuring harness holds its own lock across
  its own window.
• Torn reads. publish_lock, meta_set and remove_lock all stage and
  mv -T, so a concurrent reader sees old or new and never half.
• Placement in bench_generic. lock_before is read after warmups and
  Session::new and immediately before the measured loop; lock_after
  immediately after it. It brackets the measured window, not the setup, and it
  is computed before the --native-only split so both print paths carry it.
• Nothing in the diff writes to the real lock or to /tmp. The module only
  ever read_to_strings; the tests write solely under CARGO_TARGET_TMPDIR via
  a HOSTLOCK_DIR override, and the agreement test asserts the script refuses
  a live-anchor release rather than forcing it.

Validation after the fixes

20 unit + 2 integration in hostmon, 39 in bench_generic, 29 in
contention.rs. cargo fmt --all --check clean, clippy -D warnings clean on
all three changed crates. 23 mutants, 23 killed — the set now includes
attribute-on-sanitised (reverts finding 2) and owner-raw-sanitised
(sanitises owner_raw at parse time), both killed by
a_name_that_merely_sanitises_to_ours_is_not_ours.

Marking ready for review.

[justinchuby]

d02c76016 — one more defect, found by writing the documentation

Documenting how to make a row read mine: turned up a gap in the tool chain and
then a defect in this reader. Recording both because the second one is the kind
this PR is supposed to catch, and I introduced it.

The gap. hostlock.sh run does not export --owner. So
scripts/hostlock.sh run --owner leon -- bench_generic ... produces
host_lock=held:leon, not mine:leon — the child inherits no HOSTLOCK_OWNER
and genuinely cannot see the flag. held: is the honest answer there, and the
README now says to set the environment variable (HOSTLOCK_OWNER=leon scripts/hostlock.sh run ..., which sets both, since --owner defaults to it).

Still no fallback to $USER, though the script has one. Every agent on this
host runs as the same user, so $USER cannot distinguish one declaration from
another — defaulting to it would report mine: for a co-tenant's lock, which is
the single direction this field exists to prevent. Absent attribution stays
absent.

The defect. The test for that rule failed on its first run:

---- an_absent_owner_is_never_filled_in_from_somewhere_else ----
panicked at hostlock.rs:731: two unnamed parties are not the same party

A holder whose owner is blank has an empty owner_raw; an empty
HOSTLOCK_OWNER trims to the same; "" == "" compared equal and returned
Mine, certifying the row. An empty name on either side is the absence of an
attribution, not an attribution to nobody. Guarded — it now reports
foreign:?.

Note this is the third variant of the same mistake in this one comparison, and
each was found by a different method: comparing sanitised owners (found by the
independent review), truncation collisions (found by the same review), and now
the empty-string case (found by writing the test for a documentation claim). The
common shape is that equality on a lossy or defaultable value silently widens
mine:, and mine: is the only value that marks a row protected.

Also added to scripts/ort_ab/README.md: the host_lock= value table, and an
explicit statement that it is orthogonal to the foreign_% / sib_% columns
beside it — those measure what the host did, this records what somebody said
they were doing, and neither implies the other.

24 mutants, 24 killed, including blank-owners-match, which deletes the new
guard. 21 unit + 2 integration + 39 bench_generic + 29 contention. fmt and
clippy -D warnings clean.