acpmux: fix the host approvals daemon-lock test flake (inherited flock in parallel runs)
Flake fix for a red seen on base (never blocked): cli::host_approvals::tests::a_running_daemon_whose_socket_does_not_answer_fails_the_change failed once in a full parallel cargo test -p acpmux --lib run at c8f95c77 with did not answer (connection refused). It passes alone (5/5 with --exact at c8f95c77 and at e85bfbd2).
The shared resource
It is not a path or an env var. Each test already had its own directory, named per pid plus a per-test prefix. What the tests share is the process's open file table:
- The test takes
flockon itsdaemon.lock, checks that a held lock fails the change, thendrop(held)and expects the lock to read as free: "a lock nobody holds: no daemon". - Other lib tests spawn child processes in parallel. Between its fork and its exec, a child has a copy of every open fd, including the lock's open file description.
O_CLOEXECcloses that copy only at exec. - If a spawn lands in that window, the dropped
flockstays held for a moment.daemon_runsthen seesWouldBlockandreload_onreturns the "did not answer (connection refused)" error. The.unwrap()on that step panics with it.
The fix
Check the "lock file nobody holds" case before the test takes the lock, not after it drops it. The test still makes the same three checks (no lock file means no daemon, a lock file nobody holds means no daemon, a held lock fails the change), and they go through the same daemon_runs / reload_on code paths. No assertion is weaker. A comment on the test explains the ordering.
The two sibling tests (a_change_rewrites_the_file_whole_and_keeps_each_mac_once, a_change_holds_its_lock_until_the_reload_returns) move from per-pid temp_dir() folders to tempfile::tempdir(), so a panic no longer leaves a folder behind. They did not share a path, and the lock-hold test's 10 s wait already absorbs a brief inherited hold. Production code is unchanged.
Evidence
- Reproduction (run 40870018660), with a throwaway test that is not part of this PR:
- Deterministic: a child parked between fork and exec (
pre_execsleep). Afterdrop(held),daemon_runs= true. After the child's exec it is false. - Stress, with 8 threads spawning
truethe way sibling tests spawn processes: the old order (lock, drop, check) read a daemon 185/20000 times. The new order (check, then lock) read one 0/20000 times.
- Deterministic: a child parked between fork and exec (
- At this head (run 40870087970):
cargo clippy -p acpmux --all-targets -- -D warningspasses with no warnings.cargo test -p acpmux --lib -- --test-threads=32ran 5 times. Everyhost_approvalstest passed in all 5.- The one red in every run is
cli::hosts::tests::linux_setup_installs_the_unit_and_hands_a_detached_daemon_to_it, a known base red with its own fix in a separate PR. Run 5 also had a second failure outsidehost_approvals. My filter did not capture its name, so I can't say which test it was; it was not in the file this PR changes.
Risk: test-only, one file.
Agent session: agent session
š¤ Generated with Claude Code
![View with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3-3x.png)
![Autofix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark-3x.png)
Need help on this PR? Tag @codesmith-bot with what you need.
Summary by cubic
Fixes a flaky host_approvals test in acpmux that failed when parallel tests spawned child processes while the test held its daemon lock.
The test previously took the lock, dropped it, then asserted the lock read as free. Between a child's fork and exec, the child inherits a copy of every open file descriptor, including the dropped lock, so the assertion could briefly read a running daemon and panic with "did not answer (connection refused)". The test now checks the unheld lock file before taking the lock; all three assertions and code paths are unchanged.
- Moves the two sibling tests from per-pid temp dirs to
tempfile::tempdir()so panics don't leave folders behind. - Production code is untouched.