diff --git a/crates/fspy/src/ipc.rs b/crates/fspy/src/ipc.rs index 51d498600..43803c12e 100644 --- a/crates/fspy/src/ipc.rs +++ b/crates/fspy/src/ipc.rs @@ -4,6 +4,7 @@ use fspy_shared::ipc::{ PathAccess, channel::{Receiver, ReceiverLockGuard}, }; +#[cfg(windows)] use tokio::task::spawn_blocking; // Shared memory size for storing path accesses. @@ -22,10 +23,17 @@ pub struct OwnedReceiverLockGuard { } impl OwnedReceiverLockGuard { + #[cfg(windows)] pub fn lock(receiver: Receiver) -> io::Result { Self::try_new(receiver, fspy_shared::ipc::channel::Receiver::lock) } + #[cfg(unix)] + pub fn try_lock(receiver: Receiver) -> io::Result { + Self::try_new(receiver, fspy_shared::ipc::channel::Receiver::try_lock) + } + + #[cfg(windows)] pub async fn lock_async(receiver: Receiver) -> io::Result { spawn_blocking(move || Self::lock(receiver)).await.expect("lock task panicked") } diff --git a/crates/fspy/src/unix/mod.rs b/crates/fspy/src/unix/mod.rs index f01f63b5d..e248a90d2 100644 --- a/crates/fspy/src/unix/mod.rs +++ b/crates/fspy/src/unix/mod.rs @@ -160,9 +160,14 @@ impl SpyImpl { // Lock the ipc channel after the child has exited. // We are not interested in path accesses from descendants after the main child has exited. + // Detached descendants can keep an IPC sender alive indefinitely; in that + // case, skip IPC reads for this run instead of blocking the task runner. #[cfg(not(target_env = "musl"))] - let ipc_receiver_lock_guard = - OwnedReceiverLockGuard::lock_async(ipc_receiver).await?; + let ipc_receiver_lock_guard = match OwnedReceiverLockGuard::try_lock(ipc_receiver) { + Ok(lock) => Some(lock), + Err(err) if err.kind() == io::ErrorKind::WouldBlock => None, + Err(err) => return Err(err), + }; let path_accesses = PathAccessIterable { arenas, #[cfg(not(target_env = "musl"))] @@ -180,17 +185,30 @@ impl SpyImpl { pub struct PathAccessIterable { arenas: Vec, #[cfg(not(target_env = "musl"))] - ipc_receiver_lock_guard: OwnedReceiverLockGuard, + ipc_receiver_lock_guard: Option, } impl PathAccessIterable { + #[must_use] + pub fn is_complete(&self) -> bool { + #[cfg(not(target_env = "musl"))] + { + self.ipc_receiver_lock_guard.is_some() + } + #[cfg(target_env = "musl")] + { + true + } + } + pub fn iter(&self) -> impl Iterator> { let accesses_in_arena = self.arenas.iter().flat_map(|arena| arena.borrow_accesses().iter()).copied(); #[cfg(not(target_env = "musl"))] { - let accesses_in_shm = self.ipc_receiver_lock_guard.iter_path_accesses(); + let accesses_in_shm = + self.ipc_receiver_lock_guard.iter().flat_map(|guard| guard.iter_path_accesses()); accesses_in_shm.chain(accesses_in_arena) } #[cfg(target_env = "musl")] diff --git a/crates/fspy/src/windows/mod.rs b/crates/fspy/src/windows/mod.rs index 8081e1298..cee00c9a6 100644 --- a/crates/fspy/src/windows/mod.rs +++ b/crates/fspy/src/windows/mod.rs @@ -34,6 +34,11 @@ pub struct PathAccessIterable { } impl PathAccessIterable { + #[must_use] + pub const fn is_complete(&self) -> bool { + true + } + pub fn iter(&self) -> impl Iterator> { self.ipc_receiver_lock_guard.iter_path_accesses() } diff --git a/crates/fspy/tests/detached_child.rs b/crates/fspy/tests/detached_child.rs new file mode 100644 index 000000000..a4d20dea2 --- /dev/null +++ b/crates/fspy/tests/detached_child.rs @@ -0,0 +1,32 @@ +#![cfg(all(unix, not(target_env = "musl")))] + +use std::{process::Stdio, time::Duration}; + +use tokio::{io::AsyncReadExt as _, time::timeout}; +use tokio_util::sync::CancellationToken; + +#[test_log::test(tokio::test)] +async fn detached_descendant_sender_does_not_block_wait() -> anyhow::Result<()> { + let mut command = fspy::Command::new("/bin/sh"); + command + .arg("-c") + .arg("sleep 5 >/dev/null 2>&1 & echo $!") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + + let mut child = command.spawn(CancellationToken::new()).await?; + let mut stdout = child.stdout.take().unwrap(); + let mut child_pid = String::new(); + stdout.read_to_string(&mut child_pid).await?; + + let termination = timeout(Duration::from_secs(1), child.wait_handle).await??; + assert!(termination.status.success()); + assert!( + !termination.path_accesses.is_complete(), + "detached descendant should keep IPC sender alive past the root child" + ); + + let _ = std::process::Command::new("kill").arg(child_pid.trim()).status(); + Ok(()) +} diff --git a/crates/fspy_shared/src/ipc/channel/mod.rs b/crates/fspy_shared/src/ipc/channel/mod.rs index eb0738129..de809f92d 100644 --- a/crates/fspy_shared/src/ipc/channel/mod.rs +++ b/crates/fspy_shared/src/ipc/channel/mod.rs @@ -194,6 +194,23 @@ impl Receiver { let reader = ShmReader::new(unsafe { self.shm.as_slice() }); Ok(ReceiverLockGuard { reader, lock_file: &self.lock_file }) } + + /// Try to lock the shared memory for unique read access without blocking. + /// + /// Returns [`io::ErrorKind::WouldBlock`] when one or more senders are still + /// alive. This lets callers avoid waiting forever on detached descendants + /// that inherited or recreated a sender after the root child exited. + #[expect( + clippy::missing_errors_doc, + reason = "error conditions are self-evident from return type" + )] + pub fn try_lock(&self) -> io::Result> { + self.lock_file.try_lock()?; + // SAFETY: The exclusive file lock is held, so no writers can access the shared memory. + // The lock ensures all prior writes are visible to this thread. + let reader = ShmReader::new(unsafe { self.shm.as_slice() }); + Ok(ReceiverLockGuard { reader, lock_file: &self.lock_file }) + } } pub struct ReceiverLockGuard<'a> { diff --git a/crates/vite_task/src/session/event.rs b/crates/vite_task/src/session/event.rs index a04af70ab..98a87a78e 100644 --- a/crates/vite_task/src/session/event.rs +++ b/crates/vite_task/src/session/event.rs @@ -83,6 +83,10 @@ pub enum CacheNotUpdatedReason { /// (its `input` config includes auto-inference). Task ran but cannot /// be cached without tracked path accesses. FspyUnsupported, + /// Path tracking was enabled, but fspy could not safely read the IPC + /// buffer after the root child exited because another sender was still + /// alive. Task ran successfully, but cache was not updated. + PathTrackingUnavailable, /// The runner's IPC server failed during execution, so the collected /// reports may be incomplete. Caching such a run would risk stale /// inputs/outputs on the next hit. Carries the underlying error for diff --git a/crates/vite_task/src/session/execute/cache_update.rs b/crates/vite_task/src/session/execute/cache_update.rs index 0cf4df1c5..744cca97f 100644 --- a/crates/vite_task/src/session/execute/cache_update.rs +++ b/crates/vite_task/src/session/execute/cache_update.rs @@ -89,6 +89,16 @@ pub(super) async fn update_cache( return (CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::NonZeroExitStatus), None); } + #[cfg(fspy)] + if fspy.is_some() + && !outcome.path_accesses.as_ref().is_some_and(fspy::PathAccessIterable::is_complete) + { + return ( + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::PathTrackingUnavailable), + None, + ); + } + let fspy_outcome = observe_fspy( outcome, metadata, diff --git a/crates/vite_task/src/session/reporter/summary.rs b/crates/vite_task/src/session/reporter/summary.rs index 83bd2a937..f7880148c 100644 --- a/crates/vite_task/src/session/reporter/summary.rs +++ b/crates/vite_task/src/session/reporter/summary.rs @@ -108,6 +108,11 @@ pub enum SpawnOutcome { /// Task ran successfully but cache was not updated. #[serde(default)] fspy_unsupported: bool, + /// `true` when fspy tracking started but could not safely be collected + /// after the root child exited, usually because a detached descendant + /// still owns the IPC sender. + #[serde(default)] + path_tracking_unavailable: bool, /// Rendered message of the IPC server error that caused the cache to /// be skipped, if any. ipc_server_error: Option, @@ -321,6 +326,10 @@ impl TaskResult { cache_update_status, CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnsupported) ); + let path_tracking_unavailable = matches!( + cache_update_status, + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::PathTrackingUnavailable) + ); let ipc_server_error = match cache_update_status { CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::IpcServerError(err)) => { Some(vite_str::format!("{err}")) @@ -344,6 +353,7 @@ impl TaskResult { saved_error, input_modified_path, fspy_unsupported, + path_tracking_unavailable, ipc_server_error, tool_disabled_cache, ), @@ -357,6 +367,7 @@ impl TaskResult { saved_error, input_modified_path, fspy_unsupported, + path_tracking_unavailable, ipc_server_error, tool_disabled_cache, ), @@ -371,6 +382,7 @@ fn spawn_outcome_from_execution( saved_error: Option<&SavedExecutionError>, input_modified_path: Option, fspy_unsupported: bool, + path_tracking_unavailable: bool, ipc_server_error: Option, tool_disabled_cache: bool, ) -> SpawnOutcome { @@ -382,6 +394,7 @@ fn spawn_outcome_from_execution( infra_error: saved_error.cloned(), input_modified_path, fspy_unsupported, + path_tracking_unavailable, ipc_server_error, tool_disabled_cache, }, @@ -402,6 +415,7 @@ fn spawn_outcome_from_execution( infra_error: None, input_modified_path: None, fspy_unsupported: false, + path_tracking_unavailable: false, ipc_server_error: None, tool_disabled_cache: false, }, @@ -551,6 +565,16 @@ impl TaskResult { "→ Not cached: `input` auto-inference isn't supported on this OS. Configure `input` manually to enable caching.", ); } + // fspy started, but a detached descendant kept the IPC sender alive. + if let Self::Spawned { + outcome: SpawnOutcome::Success { path_tracking_unavailable: true, .. }, + .. + } = self + { + return Str::from( + "→ Not cached: path tracking could not finish because a child process is still running.", + ); + } match self { Self::CacheHit { saved_duration_ms } => {