From 010f3b05cc720ed47589db221633b5b2ddf3329d Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Fri, 24 Apr 2026 16:58:13 -0700 Subject: [PATCH] v0.13.1: scsi::reset() bounded by wallclock timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production incident: autorip's poll loop called scsi::reset() on a wedged BU40N USB drive. The Linux SG_SCSI_RESET ioctl blocked indefinitely (kernel SCSI subsystem waiting for a bus-wedged device to ack a reset that will never come). Caller's poll loop hung for 60+ seconds before manual intervention. scsi::reset() now spawns a detached worker for the platform-specific reset and bounds the caller's wait via mpsc::recv_timeout. Default 30 s (DEFAULT_RESET_TIMEOUT_SECS); reset_with_timeout(device, dur) exposes the bound for callers that want a different value. Returns DeviceResetFailed on timeout. Worker thread keeps running until the kernel eventually unblocks — leaks one OS thread per hard wedge, but the daemon stays responsive instead of hanging forever. Follow-up flagged for 0.13.2: USB-attached drives wedge at the USB Mass Storage layer below SCSI; SG_SCSI_RESET doesn't help. A scsi::usb_reset(path) using USBDEVFS_RESET is the proper escalation. --- CHANGELOG.md | 37 ++++++++++++++++++++++++++ Cargo.toml | 2 +- src/scsi/mod.rs | 69 +++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ce806e..5571413 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)