Skip to content

Commit cdff4d6

Browse files
wan9chiclaude
andcommitted
fix(fspy): replace the IPC file lock with an in-mapping close gate
The old quiescence protocol attached "may write" to the shared mapping but "is still writing" to a file-lock descriptor. A descendant that closes descriptors it does not recognize released the lock while keeping full write access to the mapping, so the receiver could read frames while a straggler was mutating them. Put the gate in the shared memory itself, where a writer cannot drop it while still being able to write: one atomic word admits and counts claims, and the runner's close is a single `fetch_or` at root-process exit that fences all future claims and reports whether any write was in flight. Zero in flight proves every admitted claim ran to completion and the memory is frozen; anything else means the run is conservatively not cached. Tracking now stops when the root process exits instead of waiting for lingering descendants, so a task that leaks a daemon no longer blocks the read step, and post-exit accesses are treated as what they are: racy with respect to the task's contract. Closes #544. Closes #396. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6499a5c commit cdff4d6

19 files changed

Lines changed: 1247 additions & 255 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Changelog
22

3+
- **Changed** Automatic file-access tracking stops when a task's root process exits. Accesses from leftover descendants are no longer recorded, collecting the trace no longer waits for them to exit, and a run with a traced process still mid-write at that instant is not cached ([#544](https://github.com/voidzero-dev/vite-task/issues/544), [#396](https://github.com/voidzero-dev/vite-task/issues/396), [#577](https://github.com/voidzero-dev/vite-task/pull/577)).
34
- **Fixed** `vp run` no longer fails while setting up task communication in default Codex CLI and Claude Code sandboxes that block Unix domain sockets ([#562](https://github.com/voidzero-dev/vite-task/issues/562), [#569](https://github.com/voidzero-dev/vite-task/pull/569)).
45
- **Fixed** Automatic file-access tracking now works inside coding-agent sandboxes, including the default Codex CLI and Claude Code sandboxes ([#563](https://github.com/voidzero-dev/vite-task/issues/563), [#576](https://github.com/voidzero-dev/vite-task/pull/576)).
56
- **Added** Tasks now run with `VP_RUN=1` set, so tools can tell they are running under `vp run` instead of being invoked directly ([#570](https://github.com/voidzero-dev/vite-task/pull/570)).

Cargo.lock

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/fspy/src/ipc.rs

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,4 @@
1-
use std::io;
2-
3-
use fspy_shared::ipc::{
4-
PathAccess,
5-
channel::{Receiver, ReceiverLockGuard},
6-
};
7-
use tokio::task::spawn_blocking;
8-
91
// Shared memory size for storing path accesses.
102
// 4 GiB is large enough to store path accesses in almost any realistic scenario.
113
// This doesn't allocate physical memory until it's actually used.
124
pub const SHM_CAPACITY: usize = 4 * 1024 * 1024 * 1024;
13-
14-
#[ouroboros::self_referencing]
15-
pub struct OwnedReceiverLockGuard {
16-
/// Owns the shared memory
17-
receiver: Receiver,
18-
/// Borrows the shared memory and owns the file lock
19-
#[borrows(receiver)]
20-
#[covariant]
21-
lock_guard: ReceiverLockGuard<'this>,
22-
}
23-
24-
impl OwnedReceiverLockGuard {
25-
pub fn lock(receiver: Receiver) -> io::Result<Self> {
26-
Self::try_new(receiver, fspy_shared::ipc::channel::Receiver::lock)
27-
}
28-
29-
pub async fn lock_async(receiver: Receiver) -> io::Result<Self> {
30-
spawn_blocking(move || Self::lock(receiver)).await.expect("lock task panicked")
31-
}
32-
33-
pub fn iter_path_accesses(&self) -> impl Iterator<Item = PathAccess<'_>> {
34-
self.borrow_lock_guard()
35-
.iter_frames()
36-
.map(|frame| wincode::deserialize_exact(frame).unwrap())
37-
}
38-
}

crates/fspy/src/unix/mod.rs

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ use std::{io, path::Path};
1010
use fspy_seccomp_unotify::supervisor::supervise;
1111
use fspy_shared::ipc::PathAccess;
1212
#[cfg(not(target_env = "musl"))]
13-
use fspy_shared::ipc::{NativeStr, channel::channel};
13+
use fspy_shared::ipc::{
14+
NativeStr,
15+
channel::{ChannelFrames, channel},
16+
};
1417
#[cfg(target_os = "macos")]
1518
use fspy_shared_unix::payload::Artifacts;
1619
use fspy_shared_unix::{
@@ -25,7 +28,7 @@ use tokio::task::spawn_blocking;
2528
use tokio_util::sync::CancellationToken;
2629

2730
#[cfg(not(target_env = "musl"))]
28-
use crate::ipc::{OwnedReceiverLockGuard, SHM_CAPACITY};
31+
use crate::ipc::SHM_CAPACITY;
2932
use crate::{ChildTermination, Command, TrackedChild, arena::PathAccessArena, error::SpawnError};
3033

3134
#[derive(Debug)]
@@ -40,7 +43,7 @@ pub struct SpyImpl {
4043
impl SpyImpl {
4144
/// Initialize the fs access spy by writing the preload library on disk.
4245
///
43-
/// On musl targets, we don't build a preload library
46+
/// On musl targets, we don't build a preload library;
4447
/// only seccomp-based tracking is used.
4548
pub fn init_in(#[cfg_attr(target_env = "musl", allow(unused))] dir: &Path) -> io::Result<Self> {
4649
#[cfg(not(target_env = "musl"))]
@@ -158,15 +161,24 @@ impl SpyImpl {
158161
);
159162
let arenas = arenas.collect::<Vec<_>>();
160163

161-
// Lock the ipc channel after the child has exited.
162-
// We are not interested in path accesses from descendants after the main child has exited.
164+
// Close the ipc channel now that the child has exited. We are not
165+
// interested in path accesses from descendants after the main child
166+
// has exited, and we do not wait for them either: closing is a
167+
// single atomic operation that also fences out later writers.
168+
// A close error means a traced process was still mid-write; its
169+
// frames cannot be read safely, so the run is incomplete.
163170
#[cfg(not(target_env = "musl"))]
164-
let ipc_receiver_lock_guard =
165-
OwnedReceiverLockGuard::lock_async(ipc_receiver).await?;
171+
let (shm_frames, incomplete) = ipc_receiver
172+
.close()
173+
.map_or_else(|_| (None, true), |frames| (Some(frames), false));
166174
let path_accesses = PathAccessIterable {
167175
arenas,
168176
#[cfg(not(target_env = "musl"))]
169-
ipc_receiver_lock_guard,
177+
shm_frames,
178+
#[cfg(not(target_env = "musl"))]
179+
incomplete,
180+
#[cfg(target_env = "musl")]
181+
incomplete: false,
170182
};
171183

172184
io::Result::Ok(ChildTermination { status, path_accesses })
@@ -179,8 +191,12 @@ impl SpyImpl {
179191

180192
pub struct PathAccessIterable {
181193
arenas: Vec<PathAccessArena>,
194+
/// `None` when the channel could not be frozen for reading.
182195
#[cfg(not(target_env = "musl"))]
183-
ipc_receiver_lock_guard: OwnedReceiverLockGuard,
196+
shm_frames: Option<ChannelFrames>,
197+
/// A traced process was still writing when the channel closed, so the
198+
/// recorded accesses are not a complete picture of the run.
199+
incomplete: bool,
184200
}
185201

186202
impl PathAccessIterable {
@@ -190,12 +206,20 @@ impl PathAccessIterable {
190206

191207
#[cfg(not(target_env = "musl"))]
192208
{
193-
let accesses_in_shm = self.ipc_receiver_lock_guard.iter_path_accesses();
209+
let accesses_in_shm =
210+
self.shm_frames.iter().flat_map(ChannelFrames::iter_path_accesses);
194211
accesses_in_shm.chain(accesses_in_arena)
195212
}
196213
#[cfg(target_env = "musl")]
197214
{
198215
accesses_in_arena
199216
}
200217
}
218+
219+
/// Whether tracking was cut short, which makes the accesses above an
220+
/// incomplete record of the run.
221+
#[must_use]
222+
pub const fn is_incomplete(&self) -> bool {
223+
self.incomplete
224+
}
201225
}

crates/fspy/src/windows/mod.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ use std::{
88

99
use fspy_detours_sys::{DetourCopyPayloadToProcess, DetourUpdateProcessWithDll};
1010
use fspy_shared::{
11-
ipc::{PathAccess, channel::channel},
11+
ipc::{
12+
PathAccess,
13+
channel::{ChannelFrames, channel},
14+
},
1215
windows::{PAYLOAD_ID, Payload},
1316
};
1417
use futures_util::FutureExt;
@@ -21,21 +24,29 @@ use winapi::{
2124
use winsafe::co::{CP, WC};
2225

2326
use crate::{
24-
ChildTermination, TrackedChild,
25-
command::Command,
26-
error::SpawnError,
27-
ipc::{OwnedReceiverLockGuard, SHM_CAPACITY},
27+
ChildTermination, TrackedChild, command::Command, error::SpawnError, ipc::SHM_CAPACITY,
2828
};
2929

3030
const INTERPOSE_CDYLIB: Artifact = artifact!("fspy_preload");
3131

3232
pub struct PathAccessIterable {
33-
ipc_receiver_lock_guard: OwnedReceiverLockGuard,
33+
/// `None` when the channel could not be frozen for reading.
34+
shm_frames: Option<ChannelFrames>,
35+
/// A traced process was still writing when the channel closed, so the
36+
/// recorded accesses are not a complete picture of the run.
37+
incomplete: bool,
3438
}
3539

3640
impl PathAccessIterable {
3741
pub fn iter(&self) -> impl Iterator<Item = PathAccess<'_>> {
38-
self.ipc_receiver_lock_guard.iter_path_accesses()
42+
self.shm_frames.iter().flat_map(ChannelFrames::iter_path_accesses)
43+
}
44+
45+
/// Whether tracking was cut short, which makes the accesses above an
46+
/// incomplete record of the run.
47+
#[must_use]
48+
pub const fn is_incomplete(&self) -> bool {
49+
self.incomplete
3950
}
4051
}
4152

@@ -159,10 +170,15 @@ impl SpyImpl {
159170
child.wait().await?
160171
}
161172
};
162-
// Lock the ipc channel after the child has exited.
163-
// We are not interested in path accesses from descendants after the main child has exited.
164-
let ipc_receiver_lock_guard = OwnedReceiverLockGuard::lock_async(receiver).await?;
165-
let path_accesses = PathAccessIterable { ipc_receiver_lock_guard };
173+
// Close the ipc channel now that the child has exited. We are not
174+
// interested in path accesses from descendants after the main child
175+
// has exited, and we do not wait for them either: closing is a
176+
// single atomic operation that also fences out later writers.
177+
// A close error means a traced process was still mid-write; its
178+
// frames cannot be read safely, so the run is incomplete.
179+
let (shm_frames, incomplete) =
180+
receiver.close().map_or_else(|_| (None, true), |frames| (Some(frames), false));
181+
let path_accesses = PathAccessIterable { shm_frames, incomplete };
166182

167183
io::Result::Ok(ChildTermination { status, path_accesses })
168184
})

crates/fspy/tests/test_utils/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,11 @@ pub fn assert_contains(
4949
assert_eq!(
5050
expected_mode,
5151
actual_mode,
52-
"Expected to find access to path {} with mode {:?}, but it was not found in: {:?}",
52+
"Expected to find access to path {} with mode {:?}, but it was not found in \
53+
(tracking incomplete: {}): {:?}",
5354
expected_path.display(),
5455
expected_mode,
56+
accesses.is_incomplete(),
5557
accesses.iter().collect::<Vec<_>>()
5658
);
5759
}

crates/fspy_preload_unix/src/client/mod.rs

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,23 @@ pub mod convert;
22
pub mod raw_exec;
33

44
use std::{
5-
cell::Cell, ffi::OsStr, fmt::Debug, num::NonZeroUsize, os::unix::ffi::OsStrExt as _,
6-
path::Path, sync::OnceLock,
5+
cell::Cell,
6+
ffi::OsStr,
7+
fmt::Debug,
8+
num::NonZeroUsize,
9+
os::unix::ffi::OsStrExt as _,
10+
path::Path,
11+
sync::{
12+
OnceLock,
13+
atomic::{AtomicUsize, Ordering},
14+
},
715
};
816

917
use convert::{ToAbsolutePath, ToAccessMode};
10-
use fspy_shared::ipc::{PathAccess, channel::Sender};
18+
use fspy_shared::ipc::{
19+
PathAccess,
20+
channel::{ClaimError, Sender},
21+
};
1122
use fspy_shared_unix::{
1223
exec::ExecResolveConfig,
1324
payload::EncodedPayload,
@@ -78,9 +89,16 @@ impl Client {
7889
let frame_size = NonZeroUsize::new(serialized_size)
7990
.expect("fspy: encoded PathAccess should never be empty");
8091

81-
let mut frame = ipc_sender
82-
.claim_frame(frame_size)
83-
.expect("fspy: failed to claim frame in shared memory");
92+
let _in_flight = SendInFlight::begin();
93+
let mut frame = match ipc_sender.claim_frame(frame_size) {
94+
Ok(frame) => frame,
95+
// The channel was closed because the traced root process exited.
96+
// Accesses from whatever is left behind are dropped by design.
97+
Err(ClaimError::Closed) => return Ok(()),
98+
Err(ClaimError::Capacity) => {
99+
panic!("fspy: failed to claim frame in shared memory")
100+
}
101+
};
84102
let mut writer: &mut [u8] = &mut frame;
85103
PathAccess::serialize_into(&mut writer, &path_access)?;
86104
assert_eq!(writer.len(), 0);
@@ -125,6 +143,42 @@ impl Client {
125143

126144
static CLIENT: OnceLock<Client> = OnceLock::new();
127145

146+
/// Sends that are between claiming a frame and finishing it, on any thread.
147+
static IN_FLIGHT_SENDS: AtomicUsize = AtomicUsize::new(0);
148+
149+
struct SendInFlight;
150+
151+
impl SendInFlight {
152+
fn begin() -> Self {
153+
IN_FLIGHT_SENDS.fetch_add(1, Ordering::Relaxed);
154+
Self
155+
}
156+
}
157+
158+
impl Drop for SendInFlight {
159+
fn drop(&mut self) {
160+
IN_FLIGHT_SENDS.fetch_sub(1, Ordering::Release);
161+
}
162+
}
163+
164+
/// Waits until no send is mid-frame on any thread of this process.
165+
///
166+
/// The exit interception calls this so a voluntary exit never abandons a
167+
/// claimed frame, which would make the whole run's tracking incomplete. A send
168+
/// lasts microseconds and never blocks, so this returns almost at once. The
169+
/// deadline covers a thread that died mid-send without running its drop
170+
/// (asynchronous `pthread_cancel` and the like): exit is delayed by at most
171+
/// the deadline, and the runner treats the leaked count as an incomplete run.
172+
pub fn drain_in_flight_sends() {
173+
let deadline = std::time::Instant::now() + std::time::Duration::from_millis(100);
174+
while IN_FLIGHT_SENDS.load(Ordering::Acquire) != 0 {
175+
if std::time::Instant::now() > deadline {
176+
return;
177+
}
178+
std::thread::yield_now();
179+
}
180+
}
181+
128182
// Resolving and reporting a file access can call another interposed function.
129183
// Suppress same-thread re-entry to prevent recursive access handling while
130184
// still recording accesses from other threads.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//! Drains in-flight sends before a voluntary exit.
2+
//!
3+
//! A process may call `exit` while another of its threads is between claiming
4+
//! a frame and finishing it. Dying there abandons the frame's gate guard, and
5+
//! the runner then treats the whole run's tracking as incomplete. The drain
6+
//! lasts microseconds; signals and crashes still skip it, and the runner
7+
//! handles those by not caching the run.
8+
9+
use libc::c_int;
10+
11+
use crate::{client::drain_in_flight_sends, macros::intercept};
12+
13+
intercept!(exit: unsafe extern "C" fn(status: c_int) -> !);
14+
unsafe extern "C" fn exit(status: c_int) -> ! {
15+
drain_in_flight_sends();
16+
// SAFETY: forwarding to the real libc exit with the caller's status
17+
unsafe { exit::original()(status) }
18+
}
19+
20+
intercept!(_exit: unsafe extern "C" fn(status: c_int) -> !);
21+
unsafe extern "C" fn _exit(status: c_int) -> ! {
22+
drain_in_flight_sends();
23+
// SAFETY: forwarding to the real libc _exit with the caller's status
24+
unsafe { _exit::original()(status) }
25+
}

crates/fspy_preload_unix/src/interceptions/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
mod access;
22
mod dirent;
3+
mod exit;
34
mod open;
45
mod spawn;
56
mod stat;

crates/fspy_preload_windows/src/windows/client.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ use std::{cell::SyncUnsafeCell, ffi::CStr, mem::MaybeUninit};
22

33
use fspy_detours_sys::DetourCopyPayloadToProcess;
44
use fspy_shared::{
5-
ipc::{PathAccess, channel::Sender},
5+
ipc::{
6+
PathAccess,
7+
channel::{ClaimError, Sender, WriteEncodedError},
8+
},
69
windows::{PAYLOAD_ID, Payload},
710
};
811
use winapi::{shared::minwindef::BOOL, um::winnt::HANDLE};
@@ -40,7 +43,13 @@ impl<'a> Client<'a> {
4043
let Some(sender) = &self.ipc_sender else {
4144
return;
4245
};
43-
sender.write_encoded(&access).expect("failed to send path access");
46+
match sender.write_encoded(&access) {
47+
Ok(())
48+
// The channel was closed because the traced root process exited.
49+
// Accesses from whatever is left behind are dropped by design.
50+
| Err(WriteEncodedError::Claim(ClaimError::Closed)) => {}
51+
Err(err) => panic!("failed to send path access: {err:?}"),
52+
}
4453
}
4554

4655
pub unsafe fn prepare_child_process(&self, child_handle: HANDLE) -> BOOL {

0 commit comments

Comments
 (0)