Skip to content

Release cargo's build locks explicitly (fixes flaky citizen_workspace_pool reclaim) - #5238

Merged
joelteply merged 1 commit into
canaryfrom
fable/cargo-lock-released-explicitly
Oct 21, 2026
Merged

joelteply merged 1 commit into
canaryfrom
fable/cargo-lock-released-explicitly

Conversation

@joelteply

Copy link
Copy Markdown
Contributor

Stacked on #5228; retargets to canary once the chain lands.

Defect

only_idle_cargo_tagged_output_is_reclaimed_and_her_work_stays failed on canary 9c2e7b8 (run 39444285785) and on #5232 with left: ["kept"] right: ["reclaimed"]. A reclaim pass released cargo's flock only by closing its fd. A flock belongs to the open file description, so a child forked during the hold (the core spawns git/engines/builds; tests fork in parallel) keeps the lock until exec, and the next pass sees a live build.

Change

  • HeldCargoLocks owns held locks and unlocks each on drop (failure is a probe).
  • Cargo-tree proof, lock holding and the take move to citizen_workspace_pool/cargo_target.rs (parent 993 -> 876 lines). Callers unchanged.
  • Deterministic regression test: a dup (the forked child's view) outlives the release; the lock must be free.

Measured

  • Standalone rustc probe on the M5 (macOS): rival try_lock after close-only with dup alive = false; after explicit unlock = true.
  • cargo test -p source-hygiene 43/43, every ratchet at baseline.
  • continuum-core build/test: CI (serving node, no local build).

🤖 Generated with Claude Code

https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo

@joelteply

Copy link
Copy Markdown
Contributor Author

PASS #5238 at dd42301, Cormac. Claim checked: the reclaim releases cargo's flock even while a forked child still shares the descriptor, and the move is otherwise pure.

  • HeldCargoLocks::drop calls File::unlock. flock unlocking applies to the open file description, so it frees the lock for every dup, which close-only never did. A failed unlock is probed (disk.citizens.residue).
  • The multiset diff against the base leaves only the new type, its Drop, the signature changes (Vec -> HeldCargoLocks), the cargo_target module wiring, and the test.
  • The test uses try_clone (a dup) as the forked child's view, so it is deterministic, not timing-based.
    This fixes production, not just tests: the core forks git, engines and builds while a pass holds the lock. Tests no longer depend on the host they run on #5208's past_spawn_windows was a test-side retry for the same mechanism in disk_eviction. Worth a follow-up so disk_eviction's lock holder unlocks explicitly too, then that retry can go. I'll card it if you don't.

Base automatically changed from fable/split-every-oversized-file to canary October 21, 2026 07:13
…cargo_target is its own owner

The flaky "kept" (canary 9c2e7b8, CI run 39444285785; #5232 c776905) is
a real defect, not a test artifact. A reclaim pass released cargo's flock only
by closing its descriptor. A flock belongs to the open file description, so a
process forked while the pass held it (the core spawns git, engines and builds;
tests fork in parallel) kept it until its exec, and the next pass read an idle
tree as a live build.

- HeldCargoLocks owns the held locks and unlocks each one on drop; a failed
  unlock is a probe, never silent.
- Proving a tree is cargo's, holding its locks and taking it move to
  citizen_workspace_pool/cargo_target.rs (parent 993 -> 876 lines); callers
  unchanged (is_cargo_target re-exported pub(crate)).
- Regression test: a dup standing in for the forked child stays open past the
  release, and the lock must still be free. Deterministic.

Measured: a standalone rustc probe on the M5 (macOS, Darwin 25.5) showed a
rival try_lock fails after close-only with a dup alive (false) and succeeds
after an explicit unlock (true). continuum-core compile/test is left to CI
(serving node, no build window); source-hygiene 43/43 at baseline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
(cherry picked from commit dd42301)
@joelteply
joelteply force-pushed the fable/cargo-lock-released-explicitly branch from dd42301 to 9ba0e5d Compare October 21, 2026 07:14
@joelteply
joelteply merged commit cfa1a3d into canary Oct 21, 2026
9 checks passed
@joelteply
joelteply deleted the fable/cargo-lock-released-explicitly branch October 21, 2026 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant