From 6b44c0fb8ba8d652106671ef1af53d5a24e1bf30 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Thu, 9 Jul 2026 10:46:42 +0800 Subject: [PATCH] refactor(fspy): introduce fspy_shm facade Co-authored-by: GPT-5.6 --- Cargo.lock | 11 +- Cargo.toml | 1 + crates/fspy_shared/Cargo.toml | 3 +- crates/fspy_shared/src/ipc/channel/mod.rs | 37 +---- crates/fspy_shared/src/ipc/channel/shm_io.rs | 13 +- crates/fspy_shm/.clippy.toml | 1 + crates/fspy_shm/Cargo.toml | 24 +++ crates/fspy_shm/src/lib.rs | 152 +++++++++++++++++++ 8 files changed, 204 insertions(+), 38 deletions(-) create mode 120000 crates/fspy_shm/.clippy.toml create mode 100644 crates/fspy_shm/Cargo.toml create mode 100644 crates/fspy_shm/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 0ba95ea17..004b60a97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1353,9 +1353,9 @@ dependencies = [ "bumpalo", "bytemuck", "ctor", + "fspy_shm", "native_str", "rustc-hash", - "shared_memory", "subprocess_test", "thiserror 2.0.18", "tracing", @@ -1382,6 +1382,15 @@ dependencies = [ "wincode", ] +[[package]] +name = "fspy_shm" +version = "0.0.0" +dependencies = [ + "ctor", + "shared_memory", + "subprocess_test", +] + [[package]] name = "fspy_test_bin" version = "0.0.0" diff --git a/Cargo.toml b/Cargo.toml index c734a89db..0a6c18450 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,6 +76,7 @@ fspy_detours_sys = { path = "crates/fspy_detours_sys" } fspy_preload_unix = { path = "crates/fspy_preload_unix", artifact = "cdylib", target = "target" } fspy_preload_windows = { path = "crates/fspy_preload_windows", artifact = "cdylib", target = "target" } fspy_seccomp_unotify = { path = "crates/fspy_seccomp_unotify" } +fspy_shm = { path = "crates/fspy_shm" } fspy_shared = { path = "crates/fspy_shared" } fspy_shared_unix = { path = "crates/fspy_shared_unix" } futures = "0.3.31" diff --git a/crates/fspy_shared/Cargo.toml b/crates/fspy_shared/Cargo.toml index 8dbf3a4f8..d8f6c38b7 100644 --- a/crates/fspy_shared/Cargo.toml +++ b/crates/fspy_shared/Cargo.toml @@ -11,8 +11,8 @@ bitflags = { workspace = true } bumpalo = { workspace = true } bstr = { workspace = true } bytemuck = { workspace = true, features = ["must_cast", "derive"] } +fspy_shm = { workspace = true } native_str = { workspace = true } -shared_memory = { workspace = true, features = ["logging"] } thiserror = { workspace = true } tracing = { workspace = true } uuid = { workspace = true, features = ["v4"] } @@ -26,7 +26,6 @@ winapi = { workspace = true, features = ["std"] } assert2 = { workspace = true } ctor = { workspace = true } rustc-hash = { workspace = true } -shared_memory = { workspace = true, features = ["logging"] } subprocess_test = { workspace = true } [lints] diff --git a/crates/fspy_shared/src/ipc/channel/mod.rs b/crates/fspy_shared/src/ipc/channel/mod.rs index eb0738129..fe352ee10 100644 --- a/crates/fspy_shared/src/ipc/channel/mod.rs +++ b/crates/fspy_shared/src/ipc/channel/mod.rs @@ -4,7 +4,7 @@ mod shm_io; use std::{env::temp_dir, fs::File, io, mem::MaybeUninit, ops::Deref, path::PathBuf, sync::Arc}; -use shared_memory::{Shmem, ShmemConf}; +use fspy_shm::Shm; pub use shm_io::FrameMut; use shm_io::{ShmReader, ShmWriter}; use tracing::debug; @@ -63,22 +63,11 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> { // Initialize the lock file with a unique name. let lock_file_path = temp_dir().join(format!("fspy_ipc_{}.lock", Uuid::new_v4())); - #[cfg_attr( - not(windows), - expect(unused_mut, reason = "mut required on Windows, unused on Unix") - )] - let mut conf = ShmemConf::new().size(capacity); - // On Windows, allow opening raw shared memory (without backing file) for DLL injection scenarios - #[cfg(target_os = "windows")] - { - conf = conf.allow_raw(true); - } - - let shm = conf.create().map_err(io::Error::other)?; + let shm = fspy_shm::create(capacity)?; let conf = ChannelConf { lock_file_path: lock_file_path.as_os_str().into(), - shm_id: shm.get_os_id().into(), + shm_id: shm.id().into(), shm_size: capacity, }; @@ -98,17 +87,7 @@ impl ChannelConf { let lock_file = File::open(self.lock_file_path.to_cow_os_str())?; lock_file.try_lock_shared()?; - #[cfg_attr( - not(windows), - expect(unused_mut, reason = "mut required on Windows, unused on Unix") - )] - let mut conf = ShmemConf::new().size(self.shm_size).os_id(&self.shm_id); - // On Windows, allow opening raw shared memory (without backing file) for DLL injection scenarios - #[cfg(target_os = "windows")] - { - conf = conf.allow_raw(true); - } - let shm = conf.open().map_err(io::Error::other)?; + let shm = fspy_shm::open(&self.shm_id, self.shm_size)?; // SAFETY: `shm` is a freshly opened shared memory region with valid pointer and size. // Exclusive write access is ensured by the shared file lock held by this sender. let writer = unsafe { ShmWriter::new(shm) }; @@ -117,7 +96,7 @@ impl ChannelConf { } pub struct Sender { - writer: ShmWriter, + writer: ShmWriter, lock_file_path: Box, lock_file: File, } @@ -131,7 +110,7 @@ impl Drop for Sender { } impl Deref for Sender { - type Target = ShmWriter; + type Target = ShmWriter; fn deref(&self) -> &Self::Target { &self.writer @@ -153,7 +132,7 @@ unsafe impl Sync for Sender {} pub struct Receiver { lock_file_path: PathBuf, lock_file: File, - shm: Shmem, + shm: Shm, } #[expect( @@ -175,7 +154,7 @@ impl Drop for Receiver { } impl Receiver { - fn new(lock_file_path: PathBuf, shm: Shmem) -> io::Result { + fn new(lock_file_path: PathBuf, shm: Shm) -> io::Result { let lock_file = File::create(&lock_file_path)?; Ok(Self { lock_file_path, lock_file, shm }) } diff --git a/crates/fspy_shared/src/ipc/channel/shm_io.rs b/crates/fspy_shared/src/ipc/channel/shm_io.rs index 7890043de..dd3ce5836 100644 --- a/crates/fspy_shared/src/ipc/channel/shm_io.rs +++ b/crates/fspy_shared/src/ipc/channel/shm_io.rs @@ -9,7 +9,7 @@ use std::{ }; use bytemuck::must_cast; -use shared_memory::Shmem; +use fspy_shm::Shm; use wincode::{SchemaWrite, Serialize as _, config::DefaultConfig}; // `ShmWriter` writes headers using atomic operations to prevent partial writes due to crashes, @@ -28,7 +28,7 @@ pub trait AsRawSlice { fn as_raw_slice(&self) -> *mut [u8]; } -impl AsRawSlice for Shmem { +impl AsRawSlice for Shm { fn as_raw_slice(&self) -> *mut [u8] { slice_from_raw_parts_mut(self.as_ptr(), self.len()) } @@ -665,21 +665,22 @@ mod tests { #[test] #[cfg(not(miri))] fn real_shm_across_processes() { - use shared_memory::ShmemConf; use subprocess_test::command_for_fn; const CHILD_COUNT: usize = 12; const FRAME_COUNT_EACH_CHILD: usize = 100; - let shm = ShmemConf::new().size(1024 * 1024).create().unwrap(); - let shm_name = shm.get_os_id().to_owned(); + const SHM_SIZE: usize = 1024 * 1024; + + let shm = fspy_shm::create(SHM_SIZE).unwrap(); + let shm_name = shm.id().to_owned(); let children: Vec = (0..CHILD_COUNT) .map(|child_index| { let cmd = command_for_fn!( (shm_name.clone(), child_index), |(shm_name, child_index): (String, usize)| { - let shm = ShmemConf::new().os_id(shm_name).open().unwrap(); + let shm = fspy_shm::open(&shm_name, SHM_SIZE).unwrap(); // SAFETY: `shm` is a freshly opened shared memory region with a valid // pointer and size. Concurrent write access is safe because `ShmWriter` // uses atomic operations. diff --git a/crates/fspy_shm/.clippy.toml b/crates/fspy_shm/.clippy.toml new file mode 120000 index 000000000..c7929b369 --- /dev/null +++ b/crates/fspy_shm/.clippy.toml @@ -0,0 +1 @@ +../../.non-vite.clippy.toml \ No newline at end of file diff --git a/crates/fspy_shm/Cargo.toml b/crates/fspy_shm/Cargo.toml new file mode 100644 index 000000000..39ee88ed2 --- /dev/null +++ b/crates/fspy_shm/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "fspy_shm" +version = "0.0.0" +authors.workspace = true +edition.workspace = true +license.workspace = true +publish = false +rust-version.workspace = true + +[dependencies] +shared_memory = { workspace = true, features = ["logging"] } + +[dev-dependencies] +ctor = { workspace = true } +subprocess_test = { workspace = true } + +[lints] +workspace = true + +[lib] +doctest = false + +[package.metadata.cargo-shear] +ignored = ["ctor"] diff --git a/crates/fspy_shm/src/lib.rs b/crates/fspy_shm/src/lib.rs new file mode 100644 index 000000000..0f1cea313 --- /dev/null +++ b/crates/fspy_shm/src/lib.rs @@ -0,0 +1,152 @@ +//! Behavior-neutral shared-memory facade for fspy channels. + +use std::io; + +use shared_memory::{Shmem, ShmemConf}; + +/// An owned shared-memory mapping. +pub struct Shm { + inner: Shmem, +} + +/// Creates a shared-memory mapping of `size` bytes. +/// +/// # Errors +/// +/// Returns an error if the platform cannot create or map the region. +pub fn create(size: usize) -> io::Result { + let conf = ShmemConf::new().size(size); + #[cfg(target_os = "windows")] + let conf = conf.allow_raw(true); + + let inner = conf.create().map_err(io::Error::other)?; + Ok(Shm { inner }) +} + +/// Opens the shared-memory mapping identified by `id`. +/// +/// # Errors +/// +/// Returns an error if the mapping does not exist or cannot be mapped. +pub fn open(id: &str, size: usize) -> io::Result { + let conf = ShmemConf::new().size(size).os_id(id); + #[cfg(target_os = "windows")] + let conf = conf.allow_raw(true); + + let inner = conf.open().map_err(io::Error::other)?; + Ok(Shm { inner }) +} + +#[expect(clippy::len_without_is_empty, reason = "shared-memory mappings are always non-empty")] +impl Shm { + /// Returns this mapping's opaque platform identifier. + #[must_use] + pub fn id(&self) -> &str { + self.inner.get_os_id() + } + + /// Returns the mapped length in bytes. + #[must_use] + pub fn len(&self) -> usize { + self.inner.len() + } + + /// Returns a raw pointer to the first mapped byte. + #[must_use] + pub fn as_ptr(&self) -> *mut u8 { + self.inner.as_ptr() + } + + /// Returns the mapped bytes as a shared slice. + /// + /// # Safety + /// + /// The caller must ensure that no process or thread mutates the mapping for + /// the lifetime of the returned slice. + #[must_use] + pub unsafe fn as_slice(&self) -> &[u8] { + // SAFETY: The caller upholds the same synchronization contract required by `Shmem`. + unsafe { self.inner.as_slice() } + } +} + +#[cfg(test)] +mod tests { + use std::{mem::align_of, process::Command}; + + use subprocess_test::command_for_fn; + + use super::{Shm, create, open}; + + // Page-aligned on supported macOS and Windows targets. + const SIZE: usize = 64 * 1024; + + #[test] + fn create_and_open_are_shared() { + let owner = create(SIZE).unwrap(); + assert_eq!(owner.len(), SIZE); + assert_eq!(owner.as_ptr() as usize % align_of::(), 0); + // SAFETY: No writes occur while this slice is borrowed. + assert!(unsafe { owner.as_slice() }.iter().all(|byte| *byte == 0)); + + let opened = open(owner.id(), SIZE).unwrap(); + assert_eq!(opened.id(), owner.id()); + assert_eq!(opened.len(), SIZE); + + write_byte(&owner, 0, 17); + assert_eq!(read_byte(&opened, 0), 17); + write_byte(&opened, SIZE - 1, 29); + assert_eq!(read_byte(&owner, SIZE - 1), 29); + } + + #[test] + fn mapping_is_visible_across_processes() { + let owner = create(SIZE).unwrap(); + write_byte(&owner, 0, 17); + + let command = command_for_fn!(owner.id().to_owned(), |id: String| { + let opened = open(&id, SIZE).unwrap(); + assert_eq!(read_byte(&opened, 0), 17); + write_byte(&opened, SIZE - 1, 29); + }); + assert!(Command::from(command).status().unwrap().success()); + assert_eq!(read_byte(&owner, SIZE - 1), 29); + } + + #[test] + fn owner_drop_prevents_new_opens() { + let owner = create(SIZE).unwrap(); + let id = owner.id().to_owned(); + drop(owner); + + assert!(open(&id, SIZE).is_err()); + } + + #[test] + fn opened_mapping_survives_owner_drop() { + let owner = create(SIZE).unwrap(); + let id = owner.id().to_owned(); + let opened = open(&id, SIZE).unwrap(); + write_byte(&owner, 0, 17); + drop(owner); + + // Windows keeps the named object alive while an opened view exists. + #[cfg(not(target_os = "windows"))] + assert!(open(&id, SIZE).is_err()); + assert_eq!(read_byte(&opened, 0), 17); + write_byte(&opened, SIZE - 1, 29); + assert_eq!(read_byte(&opened, SIZE - 1), 29); + } + + fn read_byte(shm: &Shm, index: usize) -> u8 { + assert!(index < shm.len()); + // SAFETY: The index is in bounds and tests synchronize all accesses. + unsafe { shm.as_ptr().add(index).read() } + } + + fn write_byte(shm: &Shm, index: usize, value: u8) { + assert!(index < shm.len()); + // SAFETY: The index is in bounds and tests synchronize all accesses. + unsafe { shm.as_ptr().add(index).write(value) }; + } +}