fix(cr-2026-04-22): recording_filename uses atomic counter for absolute uniqueness
MINOR from the batch review of 6e9ed99: SystemTime::now() alone
cannot guarantee uniqueness under tight loops — two calls in the
same clock tick can return identical secs + nanos on some OS
timing resolutions. The filename reduction from "every second"
to "every nanosecond" addresses the flagged bug but leaves a
theoretical gap.
Adds a process-lifetime AtomicU64 counter, zero-padded to 4 digits,
as the third filename component. New shape:
kon-<secs>-<nanos_in_sec>-<counter>.wav
e.g. kon-1776828000-123456789-0000.wav
Across process restarts the counter resets to 0, but the wall-clock
secs/nanos have advanced — no cross-launch collisions possible.
Within a single process, the counter guarantees uniqueness regardless
of clock behaviour.
Test strengthened from ">=32 of 64 unique" (probabilistic) to
"1024 of 1024 unique" (absolute).
This commit is contained in:
@@ -252,40 +252,57 @@ pub fn resolve_recording_path(
|
||||
Ok(recordings_dir.join(recording_filename()))
|
||||
}
|
||||
|
||||
/// Deterministic recording filename generator. Uses seconds since the
|
||||
/// epoch for human-readable sortability plus the sub-second nanosecond
|
||||
/// component so two recordings started within the same second do not
|
||||
/// collide (brief item from the 2026-04-22 code review).
|
||||
/// Deterministic recording filename generator. Combines three fields
|
||||
/// for absolute uniqueness across rapid calls:
|
||||
///
|
||||
/// Format: `kon-<secs>-<nanos_in_sec>.wav`, e.g.
|
||||
/// `kon-1776828000-123456789.wav`.
|
||||
/// - wall-clock seconds since the epoch — human-readable and
|
||||
/// sortable;
|
||||
/// - the sub-second nanosecond component — defeats same-second
|
||||
/// collisions;
|
||||
/// - a process-lifetime atomic counter — defeats even same-nanosecond
|
||||
/// collisions, which `SystemTime::now()` alone cannot guarantee
|
||||
/// (two calls in the same clock tick can return identical nanos).
|
||||
///
|
||||
/// Format: `kon-<secs>-<nanos_in_sec>-<counter>.wav`, e.g.
|
||||
/// `kon-1776828000-123456789-0000.wav`.
|
||||
fn recording_filename() -> String {
|
||||
let duration = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap_or_default();
|
||||
let secs = duration.as_secs();
|
||||
let nanos = duration.subsec_nanos();
|
||||
format!("kon-{secs}-{nanos:09}.wav")
|
||||
let counter = RECORDING_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
|
||||
format!("kon-{secs}-{nanos:09}-{counter:04}.wav")
|
||||
}
|
||||
|
||||
/// Process-lifetime monotonic counter for `recording_filename`. Starts
|
||||
/// at 0 on each Kon launch; wall-clock secs/nanos still advance across
|
||||
/// restarts, so cross-launch collisions are already impossible — the
|
||||
/// counter is the last-mile guarantee against within-launch same-tick
|
||||
/// collisions.
|
||||
static RECORDING_COUNTER: std::sync::atomic::AtomicU64 =
|
||||
std::sync::atomic::AtomicU64::new(0);
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::recording_filename;
|
||||
|
||||
#[test]
|
||||
fn recording_filenames_are_unique_across_rapid_calls() {
|
||||
// Regression for the 2026-04-22 review: two recordings
|
||||
// started in the same second previously collided on a
|
||||
// seconds-only filename. The sub-second precision added
|
||||
// here must hold under a tight loop.
|
||||
// Regression for the 2026-04-22 review AND the review-of-
|
||||
// review MINOR: SystemTime::now() alone cannot guarantee
|
||||
// uniqueness under tight loops on every OS clock resolution,
|
||||
// so the filename now includes a process-lifetime atomic
|
||||
// counter. With the counter, uniqueness is absolute across
|
||||
// any number of in-process calls.
|
||||
let mut names = std::collections::HashSet::new();
|
||||
for _ in 0..64 {
|
||||
for _ in 0..1024 {
|
||||
names.insert(recording_filename());
|
||||
}
|
||||
assert!(
|
||||
names.len() >= 32,
|
||||
"expected at least 32 unique filenames across 64 rapid calls, got {}",
|
||||
names.len()
|
||||
assert_eq!(
|
||||
names.len(),
|
||||
1024,
|
||||
"every filename must be unique (counter-backed guarantee)"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -294,16 +311,22 @@ mod tests {
|
||||
let name = recording_filename();
|
||||
assert!(name.starts_with("kon-"));
|
||||
assert!(name.ends_with(".wav"));
|
||||
// Shape: kon-<digits>-<9 digits>.wav
|
||||
// Shape: kon-<digits>-<9 digits>-<>=4 digits>.wav
|
||||
let rest = name
|
||||
.strip_prefix("kon-")
|
||||
.and_then(|s| s.strip_suffix(".wav"))
|
||||
.expect("shape prefix/suffix");
|
||||
let parts: Vec<&str> = rest.split('-').collect();
|
||||
assert_eq!(parts.len(), 2, "expected two '-' separated parts, got {parts:?}");
|
||||
assert_eq!(
|
||||
parts.len(),
|
||||
3,
|
||||
"expected three '-' separated parts, got {parts:?}"
|
||||
);
|
||||
assert!(parts[0].chars().all(|c| c.is_ascii_digit()), "secs is digits");
|
||||
assert_eq!(parts[1].len(), 9, "nanos component is zero-padded to 9 digits");
|
||||
assert!(parts[1].chars().all(|c| c.is_ascii_digit()));
|
||||
assert!(parts[2].len() >= 4, "counter component is zero-padded to >=4 digits");
|
||||
assert!(parts[2].chars().all(|c| c.is_ascii_digit()));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user