Skip to content

Commit 2788743

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 e67d3fa commit 2788743

15 files changed

Lines changed: 1048 additions & 219 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 now stops the moment a task's root process exits. Accesses made by leftover descendants afterwards are no longer recorded and never delay `vp run`, and a task that still had a traced process mid-write at that instant is conservatively 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** Automatic file-access tracking now backs its shared memory with a sparse temporary file on every platform, so tasks are still tracked inside coding-agent sandboxes that deny POSIX shared memory and Unix domain sockets ([#563](https://github.com/voidzero-dev/vite-task/issues/563), [#576](https://github.com/voidzero-dev/vite-task/pull/576)).
45
- **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)).
56
- **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)).

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: 33 additions & 9 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)]
@@ -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_preload_unix/src/client/mod.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ use std::{
77
};
88

99
use convert::{ToAbsolutePath, ToAccessMode};
10-
use fspy_shared::ipc::{PathAccess, channel::Sender};
10+
use fspy_shared::ipc::{
11+
PathAccess,
12+
channel::{ClaimError, Sender},
13+
};
1114
use fspy_shared_unix::{
1215
exec::ExecResolveConfig,
1316
payload::EncodedPayload,
@@ -78,9 +81,15 @@ impl Client {
7881
let frame_size = NonZeroUsize::new(serialized_size)
7982
.expect("fspy: encoded PathAccess should never be empty");
8083

81-
let mut frame = ipc_sender
82-
.claim_frame(frame_size)
83-
.expect("fspy: failed to claim frame in shared memory");
84+
let mut frame = match ipc_sender.claim_frame(frame_size) {
85+
Ok(frame) => frame,
86+
// The channel was closed because the traced root process exited.
87+
// Accesses from whatever is left behind are dropped by design.
88+
Err(ClaimError::Closed) => return Ok(()),
89+
Err(ClaimError::Capacity) => {
90+
panic!("fspy: failed to claim frame in shared memory")
91+
}
92+
};
8493
let mut writer: &mut [u8] = &mut frame;
8594
PathAccess::serialize_into(&mut writer, &path_access)?;
8695
assert_eq!(writer.len(), 0);

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 {

crates/fspy_shared/Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ bytemuck = { workspace = true, features = ["must_cast", "derive"] }
1414
fspy_shm = { workspace = true }
1515
native_str = { workspace = true }
1616
thiserror = { workspace = true }
17-
tracing = { workspace = true }
18-
uuid = { workspace = true, features = ["v4"] }
1917
vite_path = { workspace = true }
2018

2119
[target.'cfg(target_os = "windows")'.dependencies]
@@ -27,7 +25,6 @@ assert2 = { workspace = true }
2725
ctor = { workspace = true }
2826
rustc-hash = { workspace = true }
2927
subprocess_test = { workspace = true }
30-
tokio = { workspace = true, features = ["macros", "net", "rt-multi-thread", "time"] }
3128

3229
[lints]
3330
workspace = true

0 commit comments

Comments
 (0)