fix(version): make the update write probe collision-free #140

Merged

Claude-Run merged 1 commit from fix/YT-85-probe-filename-collision into main

2026-08-02 01:11:42 +02:00

Claude-Run commented

2026-08-02 01:10:40 +02:00

Member

check_writable built its probe filename from the pid plus SystemTime::now() nanos, so uniqueness depended on the clock ticking between calls. Under cargo test every run(...) test in commands::version::update::tests shares one pid and one parent directory and the libtest harness runs them on concurrent threads, so two threads reading the same clock tick produced the same probe name; the loser of the create_new race got EEXIST and the catch-all arm reported probe write access for <dir>: File exists (os error 17) for a directory that is in fact writable. The victim test was arbitrary, which is why the CI failure moved between runs.

probe_filename now appends a process-wide relaxed atomic counter, so two calls can never return the same string regardless of clock resolution; the pid still separates live processes and the nanos component still separates a recycled pid from a stale probe left by a killed process. check_writable additionally retries with a fresh name on ErrorKind::AlreadyExists, bounded at 3 attempts, for that residual stale-probe case; the PermissionDenied friendly message and the probe write access for <dir> context for every other error kind are unchanged, so NotFound is still reported immediately.

Tests: a new check_writable_is_safe_from_concurrent_callers runs 16 threads through 200 barrier-synchronised rounds against one shared tempfile::tempdir() and asserts every probe returned Ok (it reports 13-35 File exists failures against the pre-fix code). probe_filename_unique_per_call now asserts uniqueness across 1000 calls instead of two, so it no longer depends on the clock ticking. No test is serialised, ignored, or moved to a single-threaded runner.

#YT-85

`check_writable` built its probe filename from the pid plus `SystemTime::now()` nanos, so uniqueness depended on the clock ticking between calls. Under `cargo test` every `run(...)` test in `commands::version::update::tests` shares one pid and one parent directory and the libtest harness runs them on concurrent threads, so two threads reading the same clock tick produced the same probe name; the loser of the `create_new` race got `EEXIST` and the catch-all arm reported `probe write access for <dir>: File exists (os error 17)` for a directory that is in fact writable. The victim test was arbitrary, which is why the CI failure moved between runs. `probe_filename` now appends a process-wide relaxed atomic counter, so two calls can never return the same string regardless of clock resolution; the pid still separates live processes and the nanos component still separates a recycled pid from a stale probe left by a killed process. `check_writable` additionally retries with a fresh name on `ErrorKind::AlreadyExists`, bounded at 3 attempts, for that residual stale-probe case; the `PermissionDenied` friendly message and the `probe write access for <dir>` context for every other error kind are unchanged, so `NotFound` is still reported immediately. Tests: a new `check_writable_is_safe_from_concurrent_callers` runs 16 threads through 200 barrier-synchronised rounds against one shared `tempfile::tempdir()` and asserts every probe returned `Ok` (it reports 13-35 `File exists` failures against the pre-fix code). `probe_filename_unique_per_call` now asserts uniqueness across 1000 calls instead of two, so it no longer depends on the clock ticking. No test is serialised, ignored, or moved to a single-threaded runner. #YT-85

Claude-Run added 1 commit

2026-08-02 01:10:40 +02:00

fix(version): make the update write probe collision-free

Check / fmt + clippy + build + tests (pull_request) Successful in 45s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

fe640926fe

`check_writable` built its probe filename from the pid plus `SystemTime::now()` nanos, so uniqueness depended on the clock ticking between calls. Under `cargo test` every `run(...)` test in `commands::version::update::tests` shares one pid and one parent directory and the libtest harness runs them on concurrent threads, so two threads reading the same clock tick produced the same probe name; the loser of the `create_new` race got `EEXIST` and the catch-all arm reported `probe write access for <dir>: File exists (os error 17)` for a directory that is in fact writable. The victim test was arbitrary, which is why the CI failure moved between runs.

`probe_filename` now appends a process-wide relaxed atomic counter, so two calls can never return the same string regardless of clock resolution; the pid still separates live processes and the nanos component still separates a recycled pid from a stale probe left by a killed process. `check_writable` additionally retries with a fresh name on `ErrorKind::AlreadyExists`, bounded at 3 attempts, for that residual stale-probe case; the `PermissionDenied` friendly message and the `probe write access for <dir>` context for every other error kind are unchanged, so `NotFound` is still reported immediately.

Tests: a new `check_writable_is_safe_from_concurrent_callers` runs 16 threads through 200 barrier-synchronised rounds against one shared `tempfile::tempdir()` and asserts every probe returned `Ok` (it reports 13-35 `File exists` failures against the pre-fix code). `probe_filename_unique_per_call` now asserts uniqueness across 1000 calls instead of two, so it no longer depends on the clock ticking. No test is serialised, ignored, or moved to a single-threaded runner.

#YT-85