diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cdc5ff..c246516 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,42 @@ # Changelog +## 0.13.1 (2026-04-24) + +### `scsi::reset()` now has a hard wallclock timeout + +Production incident on a wedged BU40N USB drive: autorip's poll loop +called `scsi::reset()` and the call hung for 60+ seconds before the +operator manually intervened. Root cause: the Linux `SG_SCSI_RESET` +ioctl can block indefinitely when the kernel SCSI subsystem is waiting +on a bus-wedged device that will never ack — there's no kernel-side +timeout on this ioctl. Without an outer wallclock bound the caller's +thread is stuck in the kernel until the device unwedges (which, for a +permanently-dead USB target, may be never). + +`scsi::reset()` is now a wrapper that runs the platform-specific reset +on a detached worker thread and bounds the caller's wait via +`mpsc::recv_timeout(DEFAULT_RESET_TIMEOUT_SECS)` (30 s). Returns +`DeviceResetFailed` on timeout. The worker thread keeps running until +the kernel eventually unblocks (we can't cancel a Linux ioctl from +userspace) — this leaks one OS thread per hard wedge, an acceptable +cost for a daemon that recovers vs. one that hangs. + +- New `pub const DEFAULT_RESET_TIMEOUT_SECS: u64 = 30;` +- New `pub fn reset_with_timeout(device, Duration) -> Result<()>` for + callers that want a different bound. +- Existing `pub fn reset(device) -> Result<()>` keeps the same + signature; behaviour change is the timeout, not the API. + +### Follow-up flagged + +`SG_SCSI_RESET` only resets at the SCSI layer. For USB-attached drives +(the BU40N case), the wedge is often in the USB Mass Storage layer +*below* SCSI — `SG_SCSI_RESET` doesn't help. The proper escalation is +`USBDEVFS_RESET` (the `usbreset.c` ioctl), which re-enumerates the +device at the USB layer. Tracked for 0.13.2: a `scsi::usb_reset(path)` +that resolves sg → USB device and issues `USBDEVFS_RESET`. That would +have recovered tonight's BU40N without operator intervention. + ## 0.13.0 (2026-04-24) ### Zero English in library — typed variants for every error path diff --git a/Cargo.toml b/Cargo.toml index 94e5e9d..244b89b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libfreemkv" -version = "0.13.0" +version = "0.13.1" edition = "2024" rust-version = "1.86" license = "AGPL-3.0-only" diff --git a/src/scsi/mod.rs b/src/scsi/mod.rs index 0efcd34..514d718 100644 --- a/src/scsi/mod.rs +++ b/src/scsi/mod.rs @@ -88,9 +88,74 @@ pub fn open(device: &Path) -> Result> { } } -/// Reset a SCSI device to a known good state. Platform-specific. -/// On Linux: open/close fd cycle + TUR + SG_SCSI_RESET escalation. +/// Default upper bound on `scsi::reset()`. The platform-specific reset +/// sequence does ~15 s of bounded sleeps + ioctls in the happy path, so +/// 30 s is roughly 2× the worst-case happy time — long enough that a +/// healthy-but-slow drive isn't false-positively timed out, short enough +/// that a kernel-wedged ioctl doesn't take down the caller's poll loop +/// for minutes. +pub const DEFAULT_RESET_TIMEOUT_SECS: u64 = 30; + +/// Reset a SCSI device to a known good state, with a hard wallclock +/// bound (`DEFAULT_RESET_TIMEOUT_SECS`). On Linux: open/close fd cycle + +/// TUR + SG_SCSI_RESET escalation. +/// +/// **Why the timeout matters.** SG_SCSI_RESET is an ioctl that can block +/// indefinitely on a kernel-wedged USB target (the kernel waits for the +/// SCSI subsystem to ack the reset, which never comes from a dead-bus +/// device). Without an outer bound, the call hangs the caller's thread +/// forever — observed in production on a wedged BU40N where the autorip +/// poll loop sat in the ioctl for 60+ s. The bounded version returns +/// `DeviceResetFailed` after `DEFAULT_RESET_TIMEOUT_SECS`; the inner +/// thread keeps running until the kernel eventually unblocks it (we +/// can't cancel a Linux ioctl from userspace), so this leaks one OS +/// thread per wedge — acceptable cost for a daemon that recovers +/// instead of hanging. pub fn reset(device: &Path) -> Result<()> { + reset_with_timeout(device, std::time::Duration::from_secs(DEFAULT_RESET_TIMEOUT_SECS)) +} + +/// Reset with a caller-specified timeout. See [`reset`] for the full +/// rationale on why an outer wallclock bound is required. +pub fn reset_with_timeout(device: &Path, timeout: std::time::Duration) -> Result<()> { + let device_owned = device.to_path_buf(); + let (tx, rx) = std::sync::mpsc::channel(); + + // Detach a worker thread for the actual reset. We never `join` it — + // if the kernel ioctl is wedged, the join would block forever, which + // is the very thing we're protecting the caller from. The thread will + // exit on its own when the kernel eventually returns from the ioctl + // (or never, if the device is permanently dead — process exit cleans + // it up). + std::thread::Builder::new() + .name("scsi-reset".into()) + .spawn(move || { + let r = reset_blocking(&device_owned); + let _ = tx.send(r); + }) + .map_err(|_| Error::DeviceResetFailed { + path: device.display().to_string(), + })?; + + match rx.recv_timeout(timeout) { + Ok(result) => result, + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => Err(Error::DeviceResetFailed { + path: device.display().to_string(), + }), + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => { + // The worker thread panicked before sending. Surface as a + // reset failure rather than a generic error. + Err(Error::DeviceResetFailed { + path: device.display().to_string(), + }) + } + } +} + +/// Inner reset — runs on the worker thread. May block indefinitely if +/// the kernel's SCSI subsystem is wedged. Callers must use the bounded +/// `reset()` wrapper above; this raw function isn't exposed. +fn reset_blocking(device: &Path) -> Result<()> { #[cfg(target_os = "linux")] { linux::SgIoTransport::reset(device)