Skip to content

Commit 29fcbd6

Browse files
wan9chiclaude
andauthored
fix(fspy): unify shared memory on a sparse temp file across all platforms (#576)
## Motivation Codex CLI's and Claude Code's default sandboxes deny the primitives fspy's Unix shared memory was built on. The macOS Seatbelt profile denies `shm_open` (#563), and both sandboxes block Unix domain sockets, which the Linux memfd broker depended on. Plain files in the temp directory work under both sandboxes; the IPC lock file lives there today. This PR replaces all three platform backends with one file-backed implementation, mapped through `memmap2` on every platform. The API has three types, each one platform concept: - `ShmKeeper` is the name. `create` returns it, it carries the identifier (the backing file's absolute path), and dropping it removes the file with `remove_file`. - `ShmHandle` is the opened file. `create` returns one, so the creator never looks its own file up by name, and `open` returns one to everybody else. `map` can be called more than once. - `Mapping` is the bytes. It keeps them alive until dropped and cannot affect the name. Removal works on every platform because modern Windows deletes with POSIX semantics: the name goes away at once, [existing handles keep working](https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/ns-ntddk-_file_disposition_information_ex), and [mapped views keep the data alive](https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-createfilemappingw). CI probes on Windows Server confirmed both, with a live writable view and with an open share-delete handle. The docs reserve the right to fail the delete while a view is mapped, and Windows versions without POSIX delete do fail it, so the keeper falls back to reopening the file with `FILE_FLAG_DELETE_ON_CLOSE` and closing it. Unit tests pin the full removal semantics: name gone with a live mapping, name gone with an open handle, and the handle still mapping the same bytes afterwards. Name removal is cleanup. The channel invalidates contents inside the shared bytes, so nothing depends on removal timing. Backing files sit directly in the system temp directory as `vite-task-fspy-<uuid>.shm` with mode `0o600`; a shared subdirectory would belong to whichever user created it first and lock everyone else out. The identifier is resolved to an absolute path at creation, so a relative `TMPDIR` in the creating process cannot mislead an opener with a different working directory. There is no broker, no global object name, no tokio requirement, and no hand-written mapping code: the Windows-specific parts shrink to the sparse-file `FSCTL` and the creation flags. If the keeper's process is killed, the file stays behind: on Unix for the temp reaper, on Windows until a cleanup tool runs. It costs about as much disk as the run wrote into it. Refs #563. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 38a7569 commit 29fcbd6

17 files changed

Lines changed: 568 additions & 1208 deletions

File tree

Cargo.lock

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

Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ jsonc-parser = { version = "0.32.0", features = ["serde"] }
9393
libc = "0.2.185"
9494
libtest-mimic = "0.8.2"
9595
memmap2 = "0.9.11"
96-
memfd = "0.6.5"
9796
monostate = "1.0.2"
9897
napi = "3"
9998
napi-build = "2"

crates/fspy_shared/src/ipc/channel/mod.rs

Lines changed: 24 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -2,55 +2,22 @@
22
33
mod shm_io;
44

5-
use std::{env::temp_dir, fs::File, io, mem::MaybeUninit, ops::Deref, path::PathBuf, sync::Arc};
5+
use std::{env::temp_dir, fs::File, io, ops::Deref, path::PathBuf};
66

7-
use fspy_shm::Shm;
7+
use fspy_shm::{Mapping, ShmKeeper};
88
pub use shm_io::FrameMut;
99
use shm_io::{ShmReader, ShmWriter};
1010
use tracing::debug;
1111
use uuid::Uuid;
12-
use wincode::{
13-
SchemaRead, SchemaWrite,
14-
config::Config,
15-
error::{ReadResult, WriteResult},
16-
io::{Reader, Writer},
17-
};
12+
use wincode::{SchemaRead, SchemaWrite};
1813

1914
use super::NativeStr;
2015

21-
/// wincode schema adapter for `Arc<str>`, which is a foreign type with unsized inner.
22-
pub(crate) struct ArcStrSchema;
23-
24-
// SAFETY: Delegates to `str`'s SchemaWrite impl, preserving its size/write invariants.
25-
unsafe impl<C: Config> SchemaWrite<C> for ArcStrSchema {
26-
type Src = Arc<str>;
27-
28-
fn size_of(src: &Self::Src) -> WriteResult<usize> {
29-
<str as SchemaWrite<C>>::size_of(src)
30-
}
31-
32-
fn write(writer: impl Writer, src: &Self::Src) -> WriteResult<()> {
33-
<str as SchemaWrite<C>>::write(writer, src)
34-
}
35-
}
36-
37-
// SAFETY: Delegates to `&str`'s SchemaRead impl; dst is initialized on Ok.
38-
unsafe impl<'de, C: Config> SchemaRead<'de, C> for ArcStrSchema {
39-
type Dst = Arc<str>;
40-
41-
fn read(mut reader: impl Reader<'de>, dst: &mut MaybeUninit<Self::Dst>) -> ReadResult<()> {
42-
let s: &str = <&str as SchemaRead<'de, C>>::get(&mut reader)?;
43-
dst.write(Arc::from(s));
44-
Ok(())
45-
}
46-
}
47-
4816
/// Serializable configuration to create channel senders.
4917
#[derive(SchemaWrite, SchemaRead, Clone, Debug)]
5018
pub struct ChannelConf {
5119
lock_file_path: Box<NativeStr>,
52-
#[wincode(with = "ArcStrSchema")]
53-
shm_id: Arc<str>,
20+
shm_id: Box<NativeStr>,
5421
}
5522

5623
/// Creates a mpsc IPC channel with one receiver and a `ChannelConf` that can be passed around processes and used to create multiple senders
@@ -59,12 +26,15 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
5926
// Initialize the lock file with a unique name.
6027
let lock_file_path = temp_dir().join(format!("fspy_ipc_{}.lock", Uuid::new_v4()));
6128

62-
let shm = fspy_shm::create(capacity)?;
29+
let (keeper, handle) = fspy_shm::create(capacity)?;
30+
let mapping = handle.map()?;
6331

64-
let conf =
65-
ChannelConf { lock_file_path: lock_file_path.as_os_str().into(), shm_id: shm.id().into() };
32+
let conf = ChannelConf {
33+
lock_file_path: lock_file_path.as_os_str().into(),
34+
shm_id: keeper.id().into(),
35+
};
6636

67-
let receiver = Receiver::new(lock_file_path, shm)?;
37+
let receiver = Receiver::new(lock_file_path, keeper, mapping)?;
6838
Ok((conf, receiver))
6939
}
7040

@@ -80,16 +50,17 @@ impl ChannelConf {
8050
let lock_file = File::open(self.lock_file_path.to_cow_os_str())?;
8151
lock_file.try_lock_shared()?;
8252

83-
let shm = fspy_shm::open(&self.shm_id)?;
84-
// SAFETY: `shm` is a freshly opened shared memory region with valid pointer and size.
85-
// Exclusive write access is ensured by the shared file lock held by this sender.
86-
let writer = unsafe { ShmWriter::new(shm) };
53+
let mapping = fspy_shm::open(&self.shm_id.to_cow_os_str())?.map()?;
54+
// SAFETY: `mapping` is a freshly mapped shared memory region with valid
55+
// pointer and size. Exclusive write access is ensured by the shared
56+
// file lock held by this sender.
57+
let writer = unsafe { ShmWriter::new(mapping) };
8758
Ok(Sender { writer, lock_file, lock_file_path: self.lock_file_path.clone() })
8859
}
8960
}
9061

9162
pub struct Sender {
92-
writer: ShmWriter<Shm>,
63+
writer: ShmWriter<Mapping>,
9364
lock_file_path: Box<NativeStr>,
9465
lock_file: File,
9566
}
@@ -104,20 +75,13 @@ impl Drop for Sender {
10475
}
10576

10677
impl Deref for Sender {
107-
type Target = ShmWriter<Shm>;
78+
type Target = ShmWriter<Mapping>;
10879

10980
fn deref(&self) -> &Self::Target {
11081
&self.writer
11182
}
11283
}
11384

114-
#[cfg_attr(
115-
target_os = "windows",
116-
expect(
117-
clippy::non_send_fields_in_send_ty,
118-
reason = "`Sender` holds a shared file lock that ensures there's no reader, so `shm` can be safely written to"
119-
)
120-
)]
12185
/// SAFETY: `Sender` holds a shared file lock that ensures there's no reader, so `shm` can be safely written to.
12286
unsafe impl Send for Sender {}
12387

@@ -129,16 +93,11 @@ unsafe impl Sync for Sender {}
12993
pub struct Receiver {
13094
lock_file_path: PathBuf,
13195
lock_file: File,
132-
shm: Shm,
96+
/// Keeps the backing file's name alive for as long as senders may attach.
97+
_keeper: ShmKeeper,
98+
mapping: Mapping,
13399
}
134100

135-
#[cfg_attr(
136-
target_os = "windows",
137-
expect(
138-
clippy::non_send_fields_in_send_ty,
139-
reason = "Receiver doesn't read or write `shm`. It only passes it to `ReceiverLockGuard` under the lock"
140-
)
141-
)]
142101
/// SAFETY: `Receiver` doesn't read or write `shm`. It only passes it to `ReceiverLockGuard` under the lock.
143102
unsafe impl Send for Receiver {}
144103

@@ -154,9 +113,9 @@ impl Drop for Receiver {
154113
}
155114

156115
impl Receiver {
157-
fn new(lock_file_path: PathBuf, shm: Shm) -> io::Result<Self> {
116+
fn new(lock_file_path: PathBuf, keeper: ShmKeeper, mapping: Mapping) -> io::Result<Self> {
158117
let lock_file = File::create(&lock_file_path)?;
159-
Ok(Self { lock_file_path, lock_file, shm })
118+
Ok(Self { lock_file_path, lock_file, _keeper: keeper, mapping })
160119
}
161120

162121
/// Lock the shared memory for unique read access.
@@ -170,7 +129,7 @@ impl Receiver {
170129
self.lock_file.lock()?;
171130
// SAFETY: The exclusive file lock is held, so no writers can access the shared memory.
172131
// The lock ensures all prior writes are visible to this thread.
173-
let reader = ShmReader::new(unsafe { self.shm.as_slice() });
132+
let reader = ShmReader::new(unsafe { self.mapping.as_slice() });
174133
Ok(ReceiverLockGuard { reader, lock_file: &self.lock_file })
175134
}
176135
}

crates/fspy_shared/src/ipc/channel/shm_io.rs

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{
99
};
1010

1111
use bytemuck::must_cast;
12-
use fspy_shm::Shm;
12+
use fspy_shm::Mapping;
1313
use wincode::{SchemaWrite, Serialize as _, config::DefaultConfig};
1414

1515
// `ShmWriter` writes headers using atomic operations to prevent partial writes due to crashes,
@@ -28,7 +28,7 @@ pub trait AsRawSlice {
2828
fn as_raw_slice(&self) -> *mut [u8];
2929
}
3030

31-
impl AsRawSlice for Shm {
31+
impl AsRawSlice for Mapping {
3232
fn as_raw_slice(&self) -> *mut [u8] {
3333
slice_from_raw_parts_mut(self.as_ptr(), self.len())
3434
}
@@ -672,31 +672,24 @@ mod tests {
672672

673673
const SHM_SIZE: usize = 1024 * 1024;
674674

675-
// On Linux, `fspy_shm::create` spawns the mapping's broker onto the
676-
// ambient tokio runtime, which serves the child processes' opens.
677-
#[cfg(target_os = "linux")]
678-
let runtime = tokio::runtime::Builder::new_multi_thread()
679-
.worker_threads(1)
680-
.enable_io()
681-
.enable_time()
682-
.build()
683-
.unwrap();
684-
#[cfg(target_os = "linux")]
685-
let _guard = runtime.enter();
686-
687-
let shm = fspy_shm::create(SHM_SIZE).unwrap();
688-
let shm_name = shm.id().to_owned();
675+
let (keeper, handle) = fspy_shm::create(SHM_SIZE).unwrap();
676+
let shm_name = keeper.id().to_str().expect("test temp dir is UTF-8").to_owned();
677+
// Map before the children run. Windows keeps views coherent while they
678+
// exist at the same time; a view created after every writer exited can
679+
// observe the file before the writers' dirty pages reach it.
680+
let mapping = handle.map().unwrap();
689681

690682
let children: Vec<Child> = (0..CHILD_COUNT)
691683
.map(|child_index| {
692684
let cmd = command_for_fn!(
693685
(shm_name.clone(), child_index),
694686
|(shm_name, child_index): (String, usize)| {
695-
let shm = fspy_shm::open(&shm_name).unwrap();
696-
// SAFETY: `shm` is a freshly opened shared memory region with a valid
697-
// pointer and size. Concurrent write access is safe because `ShmWriter`
698-
// uses atomic operations.
699-
let writer = unsafe { ShmWriter::new(shm) };
687+
let mapping =
688+
fspy_shm::open(std::ffi::OsStr::new(&shm_name)).unwrap().map().unwrap();
689+
// SAFETY: `mapping` is a freshly mapped shared memory region with a
690+
// valid pointer and size. Concurrent write access is safe because
691+
// `ShmWriter` uses atomic operations.
692+
let writer = unsafe { ShmWriter::new(mapping) };
700693
for i in 0..FRAME_COUNT_EACH_CHILD {
701694
let frame_data = std::format!("{child_index} {i}");
702695
assert!(writer.try_write_frame(frame_data.as_bytes()));
@@ -714,7 +707,7 @@ mod tests {
714707

715708
// SAFETY: All child processes have exited (waited above), so no concurrent writers exist.
716709
// The shared memory is valid and fully written.
717-
let shm = unsafe { shm.as_slice() };
710+
let shm = unsafe { mapping.as_slice() };
718711
let reader = ShmReader::new(shm);
719712
let frames = reader.iter_frames().map(BStr::new).collect::<FxHashSet<&BStr>>();
720713
assert_eq!(frames.len(), CHILD_COUNT * FRAME_COUNT_EACH_CHILD);

crates/fspy_shm/Cargo.toml

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,37 +7,21 @@ license.workspace = true
77
publish = false
88
rust-version.workspace = true
99

10-
[target.'cfg(target_os = "linux")'.dependencies]
10+
[dependencies]
1111
memmap2 = { workspace = true }
12-
memfd = { workspace = true }
13-
passfd = { workspace = true, features = ["async"] }
14-
nix = { workspace = true, features = ["fs", "socket", "user"] }
15-
tokio = { workspace = true, features = ["macros", "net", "rt", "time"] }
16-
tokio-util = { workspace = true }
17-
tracing = { workspace = true }
18-
uuid = { workspace = true, features = ["v4"] }
19-
20-
[target.'cfg(target_os = "macos")'.dependencies]
21-
base64 = { workspace = true }
22-
memmap2 = { workspace = true }
23-
nix = { workspace = true, features = ["fs", "mman"] }
2412
uuid = { workspace = true, features = ["v4"] }
2513

2614
[target.'cfg(target_os = "windows")'.dependencies]
27-
uuid = { workspace = true, features = ["v4"] }
2815
windows-sys = { workspace = true, features = [
2916
"Win32_Foundation",
30-
"Win32_Security",
3117
"Win32_Storage_FileSystem",
3218
"Win32_System_IO",
3319
"Win32_System_Ioctl",
34-
"Win32_System_Memory",
3520
] }
3621

3722
[dev-dependencies]
3823
ctor = { workspace = true }
3924
subprocess_test = { workspace = true }
40-
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
4125

4226
[lints]
4327
workspace = true

0 commit comments

Comments
 (0)