diff --git a/src/error.rs b/src/error.rs index 5642d74..b5cb636 100644 --- a/src/error.rs +++ b/src/error.rs @@ -213,6 +213,14 @@ pub const E_MP4_MISSING_CODEC_PRIVATE: u16 = 9050; /// the sink would have to write 0x0, producing a structurally complete file no /// player can render. Refuse instead. pub const E_MP4_UNKNOWN_RESOLUTION: u16 = 9055; +/// The bounded durable flush did not complete within its deadline. The data is +/// NOT known to be on stable storage; the kernel will still flush on close, but +/// that is a probability, not a barrier. +pub const E_SYNC_TIMEOUT: u16 = 9056; +/// The bounded durable flush's worker thread was lost before it reported. Same +/// durability consequence as [`E_SYNC_TIMEOUT`], different cause — a caller +/// retrying a timeout should not retry this. +pub const E_SYNC_WORKER_LOST: u16 = 9057; /// READ CAPACITY returned a short or overflowing transfer. pub const E_DISC_CAPACITY_MALFORMED: u16 = 9047; @@ -557,6 +565,10 @@ pub enum Error { /// `mp4://` video track has no resolved frame dimensions. See /// [`E_MP4_UNKNOWN_RESOLUTION`]. Mp4UnknownResolution, + /// The bounded durable flush timed out. See [`E_SYNC_TIMEOUT`]. + SyncTimeout, + /// The bounded durable flush's worker was lost. See [`E_SYNC_WORKER_LOST`]. + SyncWorkerLost, PesFrameTooLarge { size: usize, }, @@ -747,6 +759,8 @@ impl Error { Error::Mp4Invalid => E_MP4_INVALID, Error::Mp4MissingCodecPrivate => E_MP4_MISSING_CODEC_PRIVATE, Error::Mp4UnknownResolution => E_MP4_UNKNOWN_RESOLUTION, + Error::SyncTimeout => E_SYNC_TIMEOUT, + Error::SyncWorkerLost => E_SYNC_WORKER_LOST, Error::PesFrameTooLarge { .. } => E_PES_FRAME_TOO_LARGE, Error::PesInvalidMagic => E_PES_INVALID_MAGIC, Error::PesTrackTooLarge { .. } => E_PES_TRACK_TOO_LARGE, @@ -957,6 +971,9 @@ impl From for std::io::Error { 3000..=3999 => std::io::ErrorKind::PermissionDenied, 4000..=4999 => std::io::ErrorKind::Other, 5000..=5999 => std::io::ErrorKind::Other, + // A stop is an interruption, not invalid data. MUST precede the + // 6000..=6999 arm — E_HALTED is 6010 and match arms are ordered. + E_HALTED => std::io::ErrorKind::Interrupted, 6000..=6999 => std::io::ErrorKind::InvalidData, 7000..=7999 => std::io::ErrorKind::PermissionDenied, 8000..=8999 => std::io::ErrorKind::Other, @@ -1006,6 +1023,10 @@ impl From for std::io::Error { | E_MP4_INVALID | E_MP4_MISSING_CODEC_PRIVATE | E_MP4_UNKNOWN_RESOLUTION => std::io::ErrorKind::InvalidData, + // Durability, not data validity: the write landed, the flush did + // not. TimedOut keeps the std kind a caller might already branch on + // while the E-code carries the distinction. + E_SYNC_TIMEOUT | E_SYNC_WORKER_LOST => std::io::ErrorKind::TimedOut, // 9030 ExtentNotUnitAligned: a malformed/non-AACS-aligned // extent was handed to the prefetch producer. 9030 => std::io::ErrorKind::InvalidInput, @@ -1708,6 +1729,8 @@ mod tests { (Error::Mp4Invalid, E_MP4_INVALID), (Error::Mp4MissingCodecPrivate, E_MP4_MISSING_CODEC_PRIVATE), (Error::Mp4UnknownResolution, E_MP4_UNKNOWN_RESOLUTION), + (Error::SyncTimeout, E_SYNC_TIMEOUT), + (Error::SyncWorkerLost, E_SYNC_WORKER_LOST), (Error::M2tsPacketMalformed, E_M2TS_PACKET_MALFORMED), (Error::ExtentNotUnitAligned, E_EXTENT_NOT_UNIT_ALIGNED), (Error::DiscCapacityMalformed, E_DISC_CAPACITY_MALFORMED), diff --git a/src/io/writeback_file/linux.rs b/src/io/writeback_file/linux.rs index 75e6737..c88cde2 100644 --- a/src/io/writeback_file/linux.rs +++ b/src/io/writeback_file/linux.rs @@ -149,14 +149,14 @@ fn bounded_failure_to_result(e: crate::io::bounded::BoundedError) -> io::Result< target: "mux", "WritebackFile::sync_all fsync timed out after 60s; data NOT durably flushed, kernel will flush on close" ); - Err(io::Error::from(io::ErrorKind::TimedOut)) + Err(crate::error::Error::SyncTimeout.into()) } crate::io::bounded::BoundedError::Halted => { tracing::warn!( target: "mux", "WritebackFile::sync_all fsync skipped (halt requested); data NOT durably flushed, kernel will flush on close" ); - Err(io::Error::from(io::ErrorKind::Interrupted)) + Err(crate::error::Error::Halted.into()) } crate::io::bounded::BoundedError::WorkerLost => { tracing::error!( @@ -166,7 +166,7 @@ fn bounded_failure_to_result(e: crate::io::bounded::BoundedError) -> io::Result< // EIO, matching the macOS sibling: a consumer distinguishing these // three failures does so on the same value on every platform. // ErrorKind::Other carries nothing a caller can branch on. - Err(io::Error::from_raw_os_error(libc::EIO)) + Err(crate::error::Error::SyncWorkerLost.into()) } } } diff --git a/src/io/writeback_file/macos.rs b/src/io/writeback_file/macos.rs index 8afadd5..ba3f570 100644 --- a/src/io/writeback_file/macos.rs +++ b/src/io/writeback_file/macos.rs @@ -129,21 +129,21 @@ fn bounded_failure_to_result(e: crate::io::bounded::BoundedError) -> io::Result< target: "mux", "WritebackFile::sync_all F_FULLFSYNC timed out after 60s; data NOT durably flushed, kernel will flush on close" ); - Err(io::Error::from(io::ErrorKind::TimedOut)) + Err(crate::error::Error::SyncTimeout.into()) } crate::io::bounded::BoundedError::Halted => { tracing::warn!( target: "mux", "WritebackFile::sync_all F_FULLFSYNC skipped (halt requested); data NOT durably flushed, kernel will flush on close" ); - Err(io::Error::from(io::ErrorKind::Interrupted)) + Err(crate::error::Error::Halted.into()) } crate::io::bounded::BoundedError::WorkerLost => { tracing::error!( target: "mux", "WritebackFile::sync_all F_FULLFSYNC worker lost before completion; data NOT durably flushed, kernel will flush on close" ); - Err(io::Error::from_raw_os_error(libc::EIO)) + Err(crate::error::Error::SyncWorkerLost.into()) } } } @@ -215,12 +215,28 @@ mod tests { "a halted F_FULLFSYNC must not be reported as a completed sync" ); + // The three arms must be DISTINGUISHABLE, not merely non-Ok. Each + // carries its own numeric code through the "E" prefix that + // `From for io::Error` mints — the only shape `io_error_code` + // recognises. A bare `ErrorKind` cannot be classified, which is how a + // user cancel here used to read as a hard I/O failure. let lost = bounded_failure_to_result(BoundedError::WorkerLost) .expect_err("a lost F_FULLFSYNC worker must be an error"); - assert_eq!( - lost.raw_os_error(), - Some(libc::EIO), - "a lost F_FULLFSYNC worker must not be reported as a completed sync" + assert!( + lost.to_string() + .starts_with(&format!("E{}", crate::error::E_SYNC_WORKER_LOST)), + "a lost worker must be identifiable, got {lost}" + ); + assert!( + timeout + .to_string() + .starts_with(&format!("E{}", crate::error::E_SYNC_TIMEOUT)), + "a timeout must be distinguishable from a lost worker, got {timeout}" + ); + assert!( + crate::error::is_halt(&halted), + "a halt must satisfy the crate's own is_halt(), or the CLI reports a \ + user cancel as a failure; got {halted}" ); } diff --git a/src/io/writeback_file/mod.rs b/src/io/writeback_file/mod.rs index a3772e6..02c7676 100644 --- a/src/io/writeback_file/mod.rs +++ b/src/io/writeback_file/mod.rs @@ -178,15 +178,19 @@ impl WritebackFile { /// is left to the kernel's normal flush-on-close path — best /// effort, but bounded. /// - /// A bounded-fsync failure (timeout / halt / lost worker) is returned as - /// an `Err` on BOTH macOS and Linux, with the same `ErrorKind` per case and - /// `EIO` for the lost worker. So `Ok(())` means the `F_FULLFSYNC` (macOS) - /// or `fsync` (Linux) completed, on either platform, and a caller needing + /// A bounded-fsync failure is returned as an `Err` on BOTH platforms, so + /// `Ok(())` means the flush completed and a caller needing /// crash-consistency can treat it as a durability barrier. /// - /// Linux used to return `Ok(())` for all three failures with only a - /// `tracing` record; that was fixed, and this doc said otherwise for - /// longer than the bug existed. + /// The three causes are DISTINGUISHABLE by numeric code, because a caller + /// should not retry a lost worker the way it retries a timeout, and must + /// not report a user cancel as a failure: + /// + /// * [`E_SYNC_TIMEOUT`](crate::error::E_SYNC_TIMEOUT) — deadline expired + /// * [`E_HALTED`](crate::error::E_HALTED) — cancelled; + /// [`is_halt`](crate::error::is_halt) recognises it + /// * [`E_SYNC_WORKER_LOST`](crate::error::E_SYNC_WORKER_LOST) — the worker + /// thread died before reporting pub fn sync_all(&mut self) -> io::Result<()> { if self.seek_count > 0 { tracing::debug!(