Repository navigation
acpmux: machine join and consent locks unlock on drop (flock flakes) - #27435
Conversation
`cli::machine_consent::tests::tags_share_enrollment_and_live_grants_and_revocation` (about 1 in 5 full lib runs) and `cli::cloud_join::tests::concurrent_join_is_refused_before_a_second_redeem` took a nonblocking flock, dropped it and took it again. A process spawned meanwhile shares the lock's open file description from its fork until its exec, so closing our descriptor left the lock held, and the next take failed with "consent update in progress" / "another machine join is in progress". The daemon hits the same race: `record_auth_result` takes the join lock and consent changes take the write lock while the daemon spawns agents and tools. Both locks now return `HeldLock`, which unlocks explicitly on drop (as the upload record and the Codex auth broker already do). Each test now holds a copy of the lock's descriptor across the drop, the way a spawned child does, so the race is taken on every run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-1288bbb2b5d2f5c6b66a
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 5889445; does not block |
|
Owned by an agent session · session |
|
Found 8 test failures on Blacksmith runners: Failures
|
|
Bead |
teamleaderleo
left a comment
There was a problem hiding this comment.
Review (gpt-6-astra, high, fast; independent Codex reviewer): SHIP at 36954f0
No correctness blockers:
- The diagnosis and fix are correct. On Unix,
File::unlock()isflock(LOCK_UN), which releases the lock held by the shared open file description even when copies from fork or dup survive. try_clone()keeps that description open across the guard's drop, so the regression is now deterministic.HeldLock's visibility allows its construction frommachine_consent.File::unlock()exists on Windows, and acquiring the lock on non-Unix still returns an error.- Callers keep their guards for the original critical sections, and none needs a bare
File. - No new clippy issue and no production harm.
Reds at this head, all base reds that fail the same way on feat-cmux-next, none this PR's:
cmux-tui SDKs / protocol contractandcmux-tui spec inventory / inventory: test_irtest_live_raw_v12_layout_and_history_contracts_are_exact, command new-pane-right.cmux-next checks: No new SwiftUI.
|
Shipped in:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Flake fix for reds seen on base:
cli::machine_consent::tests::tags_share_enrollment_and_live_grants_and_revocationfailed atmachine_consent_tests.rs:18with "consent update in progress" in 7 of 60 fullcargo test -p acpmux --libruns at base 13531e3.cli::cloud_join::tests::concurrent_join_is_refused_before_a_second_redeemfailed once in those runs atcloud_join.rs:557. Both come from #27216. Fixing base reds; never blocked.The cause
Both locks are a nonblocking
flockon a file that is taken, dropped by closing its descriptor, then taken again.flockbelongs to the open file description, and a process spawned at that moment shares the description from its fork until its exec. Other lib tests spawn processes all the time, and so does the daemon (agents, tools, login shells). Closing our descriptor therefore left the lock held for that moment, and the next take failed as "consent update in progress" or "another machine join is in progress". #27407 fixed the same race in a test ofhost_approvals.The daemon hits it too.
record_auth_resulttakes the join lock on each authenticated cloud response, and a consent change takes the write lock, both while the daemon spawns. A change from the Settings switch could fail with "in progress" when nothing else was changing consent.The fix
join_lockand the consent write lock now returnHeldLock, which unlocks explicitly on drop. The upload record (attach/upload.rs) and the Codex auth broker already do this. The unlock acts on the open file description, so it releases the lock whatever copies remain.Evidence (Linux,
cmux-ci test, 32 vCPU)cargo test -p acpmux --libtags_share_…failed 2, 4 and 1 of 20;concurrent_join_…failed 1 of 20 (1, 2, 3)cargo test -p acpmux --libmachine_consentandcloud_joinpassed 30/30 (run)cargo clippy -p acpmux --all-targets -- -D warnings,cargo fmt --checkThe other reds in those runs fail the same way at base, and each has its own PR:
protocol::tests::protocol_ir_is_current: deterministic, the spec has been stale since Add cloud machine join CLI #27216.wireguard::tests::loopback_throughput_and_shutdown(acpmux: WireGuard loopback test checks UDP release in this process (fork-copy flake) #27434).session_scope::tests::a_session_start_never_waits_for_another_to_set_the_slice(ETXTBSY).Risk: low. A drop-time
unlockon two short-lived locks.HeldLockkeeps the file open for exactly as long as theFiledid.Agent session: agent session
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes intermittent
flockfailures inacpmuxwhere machine join and consent locks appeared held after being dropped.HeldLocktype, releasing the lock for the open file description regardless of copies.Written for commit 36954f0. Summary will update on new commits.