From e921738a8b6054ba0bd3f4248ccb034db7d6ecef Mon Sep 17 00:00:00 2001 From: wan9chi Date: Fri, 24 Jul 2026 19:05:48 +0800 Subject: [PATCH] fix(fspy): reset preload sender after fork Co-authored-by: GPT-5 Codex --- CHANGELOG.md | 1 + crates/fspy/tests/fork.rs | 162 ++++++++++++++++++ .../src/client/fork_tracker/linux.rs | 95 ++++++++++ .../src/client/fork_tracker/macos.rs | 27 +++ .../src/client/fork_tracker/mod.rs | 9 + crates/fspy_preload_unix/src/client/mod.rs | 32 ++-- .../src/client/process_sender.rs | 116 +++++++++++++ 7 files changed, 422 insertions(+), 20 deletions(-) create mode 100644 crates/fspy/tests/fork.rs create mode 100644 crates/fspy_preload_unix/src/client/fork_tracker/linux.rs create mode 100644 crates/fspy_preload_unix/src/client/fork_tracker/macos.rs create mode 100644 crates/fspy_preload_unix/src/client/fork_tracker/mod.rs create mode 100644 crates/fspy_preload_unix/src/client/process_sender.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 317b3eb5..1b760dc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** Automatic file-access tracking on Linux and macOS now captures accesses from forked children that continue running without an `exec` ([#544](https://github.com/voidzero-dev/vite-task/issues/544)). - **Fixed** `vp run` no longer fails while setting up task communication in default Codex CLI and Claude Code sandboxes that block Unix-domain sockets; Unix now uses named FIFOs ([#562](https://github.com/voidzero-dev/vite-task/issues/562)). - **Fixed** The task cache now supports much larger automatically tracked input sets without hitting wincode's default 4 MiB sequence preallocation limit ([#554](https://github.com/voidzero-dev/vite-task/pull/554)). - **Fixed** npm workspace patterns beginning with `./` now discover matching packages correctly ([vite-plus#2201](https://github.com/voidzero-dev/vite-plus/issues/2201), [#547](https://github.com/voidzero-dev/vite-task/pull/547)). diff --git a/crates/fspy/tests/fork.rs b/crates/fspy/tests/fork.rs new file mode 100644 index 00000000..41ccb196 --- /dev/null +++ b/crates/fspy/tests/fork.rs @@ -0,0 +1,162 @@ +#![cfg(all(any(target_os = "linux", target_os = "macos"), not(target_env = "musl")))] + +mod test_utils; + +use std::{ffi::CString, fs, os::unix::ffi::OsStrExt as _, path::Path, time::Duration}; + +use fspy::AccessMode; +use test_log::test; +use test_utils::assert_contains; + +#[test(tokio::test)] +async fn captures_access_after_fork_without_exec() -> anyhow::Result<()> { + let tempdir = tempfile::tempdir()?; + let prime_path = tempdir.path().join("prime"); + let after_fork_path = tempdir.path().join("after-fork"); + fs::write(&prime_path, [])?; + fs::write(&after_fork_path, [])?; + + let accesses = track_fn!( + ( + prime_path.as_os_str().as_bytes().to_vec(), + after_fork_path.as_os_str().as_bytes().to_vec(), + ), + |(prime_path, after_fork_path): (Vec, Vec)| { + let prime_path = CString::new(prime_path).unwrap(); + let after_fork_path = CString::new(after_fork_path).unwrap(); + let inherited_fds = inherited_open_fds(); + let ready = new_pipe(); + let parent_alive = new_pipe(); + + // SAFETY: The process is single-threaded at this point in the test + // function, and both branches restrict post-fork work to libc calls. + let pid = unsafe { libc::fork() }; + assert_ne!(pid, -1); + + if pid == 0 { + // SAFETY: All descriptors and C strings were prepared before + // fork and are valid in the child. + unsafe { + libc::close(ready.read); + libc::close(parent_alive.write); + for fd in inherited_fds { + libc::close(fd); + } + + // The first child access must acquire a sender backed by + // this process's own lock-file description. + open_and_close(&prime_path); + write_byte(ready.write); + libc::close(ready.write); + + wait_for_eof(parent_alive.read); + libc::close(parent_alive.read); + // Without a child-owned sender, the receiver can finish + // as soon as the direct parent exits. Give it time to do + // so before making the access this test cares about. + sleep(Duration::from_millis(250)); + + open_and_close(&after_fork_path); + libc::_exit(0); + } + } + + // SAFETY: These are the parent's valid pipe endpoints. + unsafe { + libc::close(ready.write); + libc::close(parent_alive.read); + read_byte(ready.read); + libc::close(ready.read); + } + + // Keep `parent_alive.write` open. subprocess_test exits this direct + // parent as soon as the closure returns, which releases the child. + } + ) + .await?; + + assert_contains(&accesses, Path::new(&after_fork_path), AccessMode::READ); + Ok(()) +} + +#[derive(Clone, Copy)] +struct Pipe { + read: libc::c_int, + write: libc::c_int, +} + +fn new_pipe() -> Pipe { + let mut fds = [-1; 2]; + // SAFETY: `fds` points to two writable c_int values. + assert_eq!(unsafe { libc::pipe(fds.as_mut_ptr()) }, 0); + Pipe { read: fds[0], write: fds[1] } +} + +fn inherited_open_fds() -> Vec { + #[cfg(target_os = "linux")] + const FD_DIRECTORY: &str = "/proc/self/fd"; + #[cfg(target_os = "macos")] + const FD_DIRECTORY: &str = "/dev/fd"; + + let mut fds = fs::read_dir(FD_DIRECTORY) + .unwrap() + .map(|entry| entry.unwrap().file_name().to_string_lossy().parse::().unwrap()) + .filter(|fd| *fd > libc::STDERR_FILENO) + .collect::>(); + + // Reading the directory temporarily opens an fd that may appear in its own + // listing. Remove descriptors that closed with the iterator before pipes + // are created and can reuse their numbers. + fds.retain(|fd| { + // SAFETY: F_GETFD only examines the numeric descriptor. + (unsafe { libc::fcntl(*fd, libc::F_GETFD) }) != -1 + }); + fds +} + +unsafe fn open_and_close(path: &CString) { + // SAFETY: `path` is a valid NUL-terminated string. + let fd = unsafe { libc::open(path.as_ptr(), libc::O_RDONLY) }; + if fd == -1 { + // SAFETY: Immediate process termination is valid in the fork child. + unsafe { libc::_exit(10) }; + } + // SAFETY: `fd` was returned by open above. + unsafe { libc::close(fd) }; +} + +unsafe fn write_byte(fd: libc::c_int) { + let byte = [1_u8]; + // SAFETY: `byte` is readable for one byte and `fd` is the pipe write end. + if unsafe { libc::write(fd, byte.as_ptr().cast(), byte.len()) } != 1 { + // SAFETY: Immediate process termination is valid in the fork child. + unsafe { libc::_exit(11) }; + } +} + +unsafe fn read_byte(fd: libc::c_int) { + let mut byte = [0_u8]; + // SAFETY: `byte` is writable for one byte and `fd` is the pipe read end. + assert_eq!(unsafe { libc::read(fd, byte.as_mut_ptr().cast(), byte.len()) }, 1); +} + +unsafe fn wait_for_eof(fd: libc::c_int) { + let mut byte = [0_u8]; + // SAFETY: `byte` is writable for one byte and `fd` is the pipe read end. + if unsafe { libc::read(fd, byte.as_mut_ptr().cast(), byte.len()) } != 0 { + // SAFETY: Immediate process termination is valid in the fork child. + unsafe { libc::_exit(12) }; + } +} + +unsafe fn sleep(duration: Duration) { + let request = libc::timespec { + tv_sec: libc::time_t::try_from(duration.as_secs()).unwrap(), + tv_nsec: libc::c_long::from(duration.subsec_nanos()), + }; + // SAFETY: `request` is a valid timespec and no remainder is requested. + if unsafe { libc::nanosleep(&raw const request, std::ptr::null_mut()) } != 0 { + // SAFETY: Immediate process termination is valid in the fork child. + unsafe { libc::_exit(13) }; + } +} diff --git a/crates/fspy_preload_unix/src/client/fork_tracker/linux.rs b/crates/fspy_preload_unix/src/client/fork_tracker/linux.rs new file mode 100644 index 00000000..2291e4f5 --- /dev/null +++ b/crates/fspy_preload_unix/src/client/fork_tracker/linux.rs @@ -0,0 +1,95 @@ +use std::{ + num::NonZeroUsize, + ptr::NonNull, + sync::atomic::{AtomicUsize, Ordering}, +}; + +use nix::sys::mman::{MapFlags, MmapAdvise, ProtFlags, madvise, mmap_anonymous, munmap}; + +const INITIAL_GENERATION: usize = 1; +const MARKER_SIZE: NonZeroUsize = NonZeroUsize::new(size_of::()).unwrap(); + +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct ForkGeneration(usize); + +pub struct ForkTracker { + marker: NonNull, + next_generation: AtomicUsize, +} + +impl ForkTracker { + #[cfg_attr( + test, + expect(dead_code, reason = "the preload constructor is disabled in unit-test builds") + )] + pub fn new() -> nix::Result { + // SAFETY: This creates a private anonymous mapping with a nonzero size. + let marker = unsafe { + mmap_anonymous( + None, + MARKER_SIZE, + ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, + MapFlags::MAP_PRIVATE, + ) + }?; + + if let Err(error) = unsafe { + // SAFETY: `marker` identifies the mapping created above for its + // entire length. MADV_WIPEONFORK leaves the mapping in children + // but zeroes it. + madvise(marker, MARKER_SIZE.get(), MmapAdvise::MADV_WIPEONFORK) + } { + // SAFETY: The mapping is still owned here and has its original size. + let _ = unsafe { munmap(marker, MARKER_SIZE.get()) }; + return Err(error); + } + + let marker = marker.cast::(); + // SAFETY: The mapping is writable, aligned to a page boundary, and + // large enough for one AtomicUsize. + unsafe { marker.as_ptr().write(AtomicUsize::new(INITIAL_GENERATION)) }; + + Ok(Self { marker, next_generation: AtomicUsize::new(INITIAL_GENERATION) }) + } + + pub fn generation(&self) -> ForkGeneration { + let marker = self.marker(); + let current = marker.load(Ordering::Acquire); + if current != 0 { + return ForkGeneration(current); + } + + loop { + let candidate = self.next_generation.fetch_add(1, Ordering::Relaxed).wrapping_add(1); + if candidate == 0 { + continue; + } + + match marker.compare_exchange(0, candidate, Ordering::AcqRel, Ordering::Acquire) { + Ok(_) => return ForkGeneration(candidate), + Err(current) => { + debug_assert_ne!(current, 0); + return ForkGeneration(current); + } + } + } + } + + const fn marker(&self) -> &AtomicUsize { + // SAFETY: `self.marker` remains mapped for the lifetime of `self`. + unsafe { self.marker.as_ref() } + } +} + +impl Drop for ForkTracker { + fn drop(&mut self) { + // SAFETY: `self.marker` owns the mapping with this exact size. + let _ = unsafe { munmap(self.marker.cast(), MARKER_SIZE.get()) }; + } +} + +// SAFETY: The mapping contains one AtomicUsize and all access uses atomic +// operations. The mapping address remains stable for the tracker's lifetime. +unsafe impl Send for ForkTracker {} +// SAFETY: See the Send implementation; concurrent access is atomic. +unsafe impl Sync for ForkTracker {} diff --git a/crates/fspy_preload_unix/src/client/fork_tracker/macos.rs b/crates/fspy_preload_unix/src/client/fork_tracker/macos.rs new file mode 100644 index 00000000..788548b1 --- /dev/null +++ b/crates/fspy_preload_unix/src/client/fork_tracker/macos.rs @@ -0,0 +1,27 @@ +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct ForkGeneration(libc::pid_t); + +pub struct ForkTracker; + +impl ForkTracker { + #[expect( + clippy::unnecessary_wraps, + reason = "keeps construction identical to the fallible Linux fork tracker" + )] + #[cfg_attr( + test, + expect(dead_code, reason = "the preload constructor is disabled in unit-test builds") + )] + pub const fn new() -> nix::Result { + Ok(Self) + } + + #[expect( + clippy::unused_self, + reason = "keeps generation lookup identical to the stateful Linux fork tracker" + )] + pub fn generation(&self) -> ForkGeneration { + // SAFETY: getpid has no preconditions and always succeeds. + ForkGeneration(unsafe { libc::getpid() }) + } +} diff --git a/crates/fspy_preload_unix/src/client/fork_tracker/mod.rs b/crates/fspy_preload_unix/src/client/fork_tracker/mod.rs new file mode 100644 index 00000000..d7ba2ed3 --- /dev/null +++ b/crates/fspy_preload_unix/src/client/fork_tracker/mod.rs @@ -0,0 +1,9 @@ +#[cfg(target_os = "linux")] +mod linux; +#[cfg(target_os = "macos")] +mod macos; + +#[cfg(target_os = "linux")] +pub(super) use linux::{ForkGeneration, ForkTracker}; +#[cfg(target_os = "macos")] +pub(super) use macos::{ForkGeneration, ForkTracker}; diff --git a/crates/fspy_preload_unix/src/client/mod.rs b/crates/fspy_preload_unix/src/client/mod.rs index daae12f5..1a28363e 100644 --- a/crates/fspy_preload_unix/src/client/mod.rs +++ b/crates/fspy_preload_unix/src/client/mod.rs @@ -1,30 +1,36 @@ pub mod convert; pub mod raw_exec; +mod fork_tracker; +mod process_sender; + use std::{ cell::Cell, ffi::OsStr, fmt::Debug, num::NonZeroUsize, os::unix::ffi::OsStrExt as _, path::Path, sync::OnceLock, }; use convert::{ToAbsolutePath, ToAccessMode}; -use fspy_shared::ipc::{PathAccess, channel::Sender}; +use fspy_shared::ipc::PathAccess; use fspy_shared_unix::{ exec::ExecResolveConfig, payload::EncodedPayload, spawn::{PreExec, handle_exec}, }; +use process_sender::ProcessSender; use raw_exec::RawExec; use wincode::Serialize as _; pub struct Client { encoded_payload: EncodedPayload, - ipc_sender: Option, + ipc_sender: ProcessSender, } -// SAFETY: Client fields are only mutated during initialization in the ctor; after that, all access is read-only +// SAFETY: ProcessSender publishes immutable sender states through atomics, and +// the remaining Client state is read-only after initialization. #[cfg(target_os = "macos")] unsafe impl Sync for Client {} -// SAFETY: Client is only sent once during initialization; after that it lives in a static OnceLock +// SAFETY: Client is moved into the static OnceLock during initialization. Its +// ProcessSender and read-only payload may then be shared between threads. #[cfg(target_os = "macos")] unsafe impl Send for Client {} @@ -35,32 +41,18 @@ impl Debug for Client { } impl Client { - #[expect( - clippy::print_stderr, - reason = "preload library intentionally uses stderr for error reporting" - )] #[cfg(not(test))] fn from_env() -> Self { use fspy_shared_unix::payload::decode_payload_from_env; let encoded_payload = decode_payload_from_env().unwrap(); - - let ipc_sender = match encoded_payload.payload.ipc_channel_conf.sender() { - Ok(sender) => Some(sender), - Err(err) => { - // this can happen if the process is started after the root target process has exited. - // By that time the channel would have been closed in the receiver side. - // In this case we just leave a message and skip sending any path accesses. - eprintln!("fspy: failed to create ipc sender: {err}"); - None - } - }; + let ipc_sender = ProcessSender::new(&encoded_payload.payload.ipc_channel_conf).unwrap(); Self { encoded_payload, ipc_sender } } fn send(&self, mode: fspy_shared::ipc::AccessMode, path: &Path) -> anyhow::Result<()> { - let Some(ipc_sender) = &self.ipc_sender else { + let Some(ipc_sender) = self.ipc_sender.sender() else { // ipc channel not available, skip sending return Ok(()); }; diff --git a/crates/fspy_preload_unix/src/client/process_sender.rs b/crates/fspy_preload_unix/src/client/process_sender.rs new file mode 100644 index 00000000..7c8b470c --- /dev/null +++ b/crates/fspy_preload_unix/src/client/process_sender.rs @@ -0,0 +1,116 @@ +use std::{ + io, + sync::atomic::{AtomicPtr, Ordering}, +}; + +use fspy_shared::ipc::channel::{ChannelConf, Sender}; + +use super::fork_tracker::{ForkGeneration, ForkTracker}; + +struct SenderState { + generation: ForkGeneration, + sender: Option, +} + +pub(super) struct ProcessSender { + conf: ChannelConf, + fork_tracker: ForkTracker, + state: AtomicPtr, +} + +impl ProcessSender { + #[cfg_attr( + test, + expect(dead_code, reason = "the preload constructor is disabled in unit-test builds") + )] + pub(super) fn new(conf: &ChannelConf) -> nix::Result { + let fork_tracker = ForkTracker::new()?; + let generation = fork_tracker.generation(); + let state = Box::into_raw(Box::new(SenderState::new(generation, conf))); + + Ok(Self { conf: conf.clone(), fork_tracker, state: AtomicPtr::new(state) }) + } + + pub(super) fn sender(&self) -> Option<&Sender> { + loop { + let generation = self.fork_tracker.generation(); + let current_ptr = self.state.load(Ordering::Acquire); + // SAFETY: Published states are never reclaimed, so the pointer + // remains valid for the process lifetime. + let current = unsafe { &*current_ptr }; + if current.generation == generation { + return current.sender.as_ref(); + } + + let replacement_ptr = Box::into_raw(Box::new(SenderState::new(generation, &self.conf))); + match self.state.compare_exchange( + current_ptr, + replacement_ptr, + Ordering::AcqRel, + Ordering::Acquire, + ) { + Ok(_) => { + // Do not reclaim `current_ptr`. After fork it can own an + // inherited sender whose destructor must not affect the + // parent's file description. In the original process, + // another thread may still be borrowing the old state. + // SAFETY: This state was just allocated and published, and + // published states are never reclaimed. + return unsafe { &*replacement_ptr }.sender.as_ref(); + } + Err(_) => { + // SAFETY: This allocation was never published, so no other + // thread can reference it. + drop(unsafe { Box::from_raw(replacement_ptr) }); + } + } + } + } +} + +impl SenderState { + #[expect( + clippy::print_stderr, + reason = "preload library intentionally reports sender initialization failures" + )] + fn new(generation: ForkGeneration, conf: &ChannelConf) -> Self { + let sender = match retry_interrupted(|| conf.sender()) { + Ok(sender) => Some(sender), + Err(error) => { + // The receiver may already be closed when a late child first + // accesses the filesystem. Keep the traced process running and + // cache the failure for this process generation. + eprintln!("fspy: failed to create ipc sender: {error}"); + None + } + }; + Self { generation, sender } + } +} + +fn retry_interrupted(mut operation: impl FnMut() -> io::Result) -> io::Result { + loop { + match operation() { + Err(error) if error.kind() == io::ErrorKind::Interrupted => {} + result => return result, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn interrupted_sender_creation_is_retried() { + let mut attempts = 0; + let value = retry_interrupted(|| { + attempts += 1; + if attempts < 3 { Err(io::ErrorKind::Interrupted.into()) } else { Ok(42) } + }) + .unwrap(); + + assert_eq!(value, 42); + assert_eq!(attempts, 3); + } +}