hardlink_clone_into's reader_lock_acquire is unpinned: removing it leaves every suite green at the identical 129 assertions #10

New Issue

2026-08-24T18:59:49Z

claude commented

2026-08-24 18:59:49 +00:00

Found by a mutation sweep while closing #5 (PR #9), and pre-existing — not
introduced by that PR.

Problem

Delete this line from hardlink_clone_into (scripts/cache-lib.sh:399):

reader_lock_acquire "$parent" "$src_name" "$tag"

and all five suites pass, at the identical 129 assertions — 49 / 24 / 37 /
5 / 14, byte-for-byte the unmutated counts.

Its sibling one line later is pinned: removing reader_lock_release reddens
both seed-target-dir-selftest.sh and publish-snapshot-selftest.sh (the
marker is never cleared, so the publisher's drain wait runs to its grace
timeout). Only the acquisition is unguarded.

Why it matters

This is the consumer's half of the seed-vs-republish interlock, sitting
directly under the comment that calls out its ordering as the load-bearing
part:

#   Consumer:  create .reading-<snap>-<tag>  ->  stat <snap>  ->  cp -al
#   Publisher: mv <snap> aside  ->  mv new into place  ->  scan for markers
#              ->  unlink the rotated-away generation

Without the marker, a publisher that finishes its swap while a consumer is
mid-walk unlinks the generation being read, which is the silent truncation
mode — cp -al never sees the subtree and reports nothing. The retry loop
still catches it after the fact, so this is a defence-in-depth line rather
than the only thing standing between a job and a partial cache. It is,
however, exactly the line the whole reader-marker section exists to justify.

The trap: it is exercised, but not asserted

This is not dormant code that no fixture reaches. In scenario 8a the publisher
genuinely blocks on this consumer's marker — with the line present:

readers: waiting for 1 in-flight clone(s) of snapshot-dev-34c6fcec (grace 300s)
readers: snapshot-dev-34c6fcec drained after 1s

With the line removed, those log lines disappear, the publisher never waits at
all — and scenario 8a still passes, with the same assertions and the same
verdict. Nothing reads the publisher's log for evidence that the interlock
engaged, so the interlock can stop engaging silently. Compare assert_tear,
which exists precisely so a tear scenario cannot pass without naming which
check caught it.

Ask

A scenario that fails if reader_lock_acquire is removed from
hardlink_clone_into. The cheap version is an assertion on the publisher's
log inside 8a — the wait is deterministic there by construction, since 8a's
stub holds the consumer inside cp until the swap is on disk while the marker
is held until the identity read that follows it.

Note the design tension before choosing that: 8a currently pins exactly one
property and is reddened by exactly one mutant, and the suite's
one-scenario-one-mutant diagonal is a deliberate property. Folding a second
assertion into 8a would cost that. A separate scenario for the interlock —
next to, or paired with, the publisher-side half already in
publish-snapshot-selftest.sh scenarios 6 to 8 — keeps the diagonal intact.

Acceptance criteria

A scenario fails if reader_lock_acquire is removed from
hardlink_clone_into, and passes with it. Demonstrate the red/green
explicitly rather than asserting it.
It fails for its own reason: no other mutation of cache-lib.sh's
guards reddens it, and it does not disturb the existing
one-scenario-one-mutant diagonal across scenarios 8a to 8d and 10.
It asserts that the interlock actually engaged — the publisher observably
waited on this consumer's marker — rather than that the run merely
succeeded.
The scenario's comment states which property it pins.

Also worth recording: `write_cache_lock` has no coverage

Removing the write_cache_lock "$tmp" "$lock_id" call from
hardlink_clone_into also leaves every suite green, but as an equivalent
mutant rather than an unpinned guard: no selftest passes seed-target-dir.sh
its optional 6th argument, so LOCK_ID is always empty and the function
early-returns at [ -n "$id" ] || return 0. The .ci-lock-* files that
prune-cache-selftest.sh and publish-snapshot-selftest.sh use are
hand-written fixtures, not output of this function.

So the documented property — that a seeded directory carries its lock the
instant it appears under its final name, closing the window where a concurrent
prune could evict a directory that exists but is not yet held — is untested
end to end. Lower priority than the marker above, since it needs a fixture
that passes a lock id at all before any mutation of it means anything.

Found by a mutation sweep while closing #5 (PR #9), and **pre-existing** — not introduced by that PR. ## Problem Delete this line from `hardlink_clone_into` (`scripts/cache-lib.sh:399`): ```bash reader_lock_acquire "$parent" "$src_name" "$tag" ``` and **all five suites pass, at the identical 129 assertions** — 49 / 24 / 37 / 5 / 14, byte-for-byte the unmutated counts. Its sibling one line later is pinned: removing `reader_lock_release` reddens both `seed-target-dir-selftest.sh` and `publish-snapshot-selftest.sh` (the marker is never cleared, so the publisher's drain wait runs to its grace timeout). Only the acquisition is unguarded. ## Why it matters This is the consumer's half of the seed-vs-republish interlock, sitting directly under the comment that calls out its ordering as the load-bearing part: ``` # Consumer: create .reading-<snap>-<tag> -> stat <snap> -> cp -al # Publisher: mv <snap> aside -> mv new into place -> scan for markers # -> unlink the rotated-away generation ``` Without the marker, a publisher that finishes its swap while a consumer is mid-walk unlinks the generation being read, which is the *silent* truncation mode — `cp -al` never sees the subtree and reports nothing. The retry loop still catches it after the fact, so this is a defence-in-depth line rather than the only thing standing between a job and a partial cache. It is, however, exactly the line the whole reader-marker section exists to justify. ## The trap: it is exercised, but not asserted This is not dormant code that no fixture reaches. In scenario 8a the publisher genuinely blocks on this consumer's marker — with the line present: ``` readers: waiting for 1 in-flight clone(s) of snapshot-dev-34c6fcec (grace 300s) readers: snapshot-dev-34c6fcec drained after 1s ``` With the line removed, those log lines disappear, the publisher never waits at all — and scenario 8a still passes, with the same assertions and the same verdict. Nothing reads the publisher's log for evidence that the interlock engaged, so the interlock can stop engaging silently. Compare `assert_tear`, which exists precisely so a tear scenario cannot pass without naming which check caught it. ## Ask A scenario that fails if `reader_lock_acquire` is removed from `hardlink_clone_into`. The cheap version is an assertion on the publisher's log inside 8a — the wait is deterministic there by construction, since 8a's stub holds the consumer inside `cp` until the swap is on disk while the marker is held until the identity read that follows it. Note the design tension before choosing that: 8a currently pins exactly one property and is reddened by exactly one mutant, and the suite's one-scenario-one-mutant diagonal is a deliberate property. Folding a second assertion into 8a would cost that. A separate scenario for the interlock — next to, or paired with, the publisher-side half already in `publish-snapshot-selftest.sh` scenarios 6 to 8 — keeps the diagonal intact. ## Acceptance criteria - [ ] A scenario fails if `reader_lock_acquire` is removed from `hardlink_clone_into`, and passes with it. **Demonstrate the red/green explicitly** rather than asserting it. - [ ] It fails for its own reason: no other mutation of `cache-lib.sh`'s guards reddens it, and it does not disturb the existing one-scenario-one-mutant diagonal across scenarios 8a to 8d and 10. - [ ] It asserts that the interlock actually engaged — the publisher observably waited on this consumer's marker — rather than that the run merely succeeded. - [ ] The scenario's comment states which property it pins. ## Also worth recording: `write_cache_lock` has no coverage Removing the `write_cache_lock "$tmp" "$lock_id"` call from `hardlink_clone_into` also leaves every suite green, but as an **equivalent mutant** rather than an unpinned guard: no selftest passes `seed-target-dir.sh` its optional 6th argument, so `LOCK_ID` is always empty and the function early-returns at `[ -n "$id" ] || return 0`. The `.ci-lock-*` files that `prune-cache-selftest.sh` and `publish-snapshot-selftest.sh` use are hand-written fixtures, not output of this function. So the documented property — that a seeded directory carries its lock the instant it appears under its final name, closing the window where a concurrent prune could evict a directory that exists but is not yet held — is untested end to end. Lower priority than the marker above, since it needs a fixture that passes a lock id at all before any mutation of it means anything.

claude added the bug label 2026-08-24 18:59:49 +00:00

claude referenced this issue

2026-08-24 19:03:14 +00:00

test(seed): pin the two unguarded terms of the torn-clone condition #9

claude-reviewer referenced this issue

2026-08-24 19:23:11 +00:00

test(seed): pin the two unguarded terms of the torn-clone condition #9

claude commented

2026-08-24 19:27:13 +00:00

Three more undriven paths, from a second sweep during review of #9. Same class
as the write_cache_lock note above — recorded here rather than given
scenarios, because each needs a fixture that can reach the failure at all
before mutating it means anything.

All five suites stay green at the identical 129 assertions with any of
these removed:

mutant
`unshare_subtree "$d" \|\| { … return 1; }` → `\|\| true`	`cache-lib.sh:203-206`
the `.rustc_info.json` propagation → `\|\| true`	`cache-lib.sh:212-215`
loop-top `rm -rf "$tmp"` deleted	`cache-lib.sh:395`

Why nothing drives them:

The subtree-loop propagation needs unshare_subtree to fail. It runs in
several scenarios, but seed-target-dir-selftest.sh scenario 10's stub
matches cp -p -- only, and unshare_subtree calls cp -a.
The .rustc_info.json propagation needs that copy to fail. The file
exists only in hardlink-clone-selftest.sh's real cargo build, which asserts
it gets unshared but never makes copying it fail.
The loop-top rm -rf "$tmp" is only load-bearing when a .stage-<tag>
already exists at loop entry; otherwise it is a no-op. Retries do not drive
it, because a torn attempt already removes $tmp before looping. It would
take a stale staging directory under the same tag — which no fixture
creates, and which per-job-per-run tags make unreachable in production.

So unshare_mutable_paths has three failure propagations: the .d pass is
pinned by scenario 10, the other two are undriven.

Worth pairing with the marker work above if anyone takes this on: a fixture
that can fail cp -a inside unshare_subtree would cover the first two at
once, and the third is a one-line fixture (pre-create .stage-<tag> before a
same-tag clone) rather than a scenario.

Note on provenance, since it bears on how much to trust this list: the residue
in #9 was rewritten twice and was still incomplete both times. Each round of
missing entries was found by an independent sweep, not by re-reading the
record. Treat the list as what has been checked, not as what exists.

Three more undriven paths, from a second sweep during review of #9. Same class as the `write_cache_lock` note above — recorded here rather than given scenarios, because each needs a fixture that can reach the failure at all before mutating it means anything. All five suites stay green at the identical **129 assertions** with any of these removed: | mutant | | |---|---| | `unshare_subtree "$d" \|\| { … return 1; }` → `\|\| true` | `cache-lib.sh:203-206` | | the `.rustc_info.json` propagation → `\|\| true` | `cache-lib.sh:212-215` | | loop-top `rm -rf "$tmp"` deleted | `cache-lib.sh:395` | Why nothing drives them: - The **subtree-loop** propagation needs `unshare_subtree` to fail. It runs in several scenarios, but `seed-target-dir-selftest.sh` scenario 10's stub matches `cp -p --` only, and `unshare_subtree` calls `cp -a`. - The **`.rustc_info.json`** propagation needs that copy to fail. The file exists only in `hardlink-clone-selftest.sh`'s real cargo build, which asserts it gets unshared but never makes copying it fail. - The **loop-top `rm -rf "$tmp"`** is only load-bearing when a `.stage-<tag>` already exists at loop entry; otherwise it is a no-op. Retries do not drive it, because a torn attempt already removes `$tmp` before looping. It would take a stale staging directory under the *same* tag — which no fixture creates, and which per-job-per-run tags make unreachable in production. So `unshare_mutable_paths` has three failure propagations: the `.d` pass is pinned by scenario 10, the other two are undriven. Worth pairing with the marker work above if anyone takes this on: a fixture that can fail `cp -a` inside `unshare_subtree` would cover the first two at once, and the third is a one-line fixture (pre-create `.stage-<tag>` before a same-tag clone) rather than a scenario. Note on provenance, since it bears on how much to trust this list: the residue in #9 was rewritten twice and was still incomplete both times. Each round of missing entries was found by an independent sweep, not by re-reading the record. Treat the list as what has been checked, not as what exists.

claude referenced this issue from a commit

2026-08-24 22:14:29 +00:00

test(seed): pin reader_lock_acquire in hardlink_clone_into

claude referenced a pull request that will close this issue

2026-08-24 22:15:16 +00:00

test(seed): pin reader_lock_acquire in hardlink_clone_into (#12) #12

claude-reviewer referenced this issue

2026-08-24 22:34:35 +00:00

test(seed): pin reader_lock_acquire in hardlink_clone_into (#12) #12

claude closed this issue

2026-08-24 23:05:18 +00:00

Sign in to join this conversation.