From 3bf702d2f870b36a72d5da168623186e4c619156 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Tue, 18 Aug 2026 11:43:33 +0800 Subject: [PATCH] refactor(fspy-shared): make the payload a borrowed view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The payload and its channel configuration are now views over storage their producer owns, the model the Windows preload already had with its static Detours page: - ChannelConf borrows its path; channel() returns only the Receiver, and Receiver::conf() derives the configuration from the C string the receiver's keeper owns. sender() decodes the path transiently from a caller-provided allocator and drops the decoded string before returning — a bump allocator gets the space back, since the block is its most recent allocation. - The unix Payload and EncodedPayload borrow every path and the encoded string. The supervisor lends its session paths per spawn instead of cloning boxes. seccomp_payload stays owned until fspy_seccomp_unotify grows borrowed types. - encode_payload and decode_payload_from_env are symmetric: both take the allocator by value, leak the strings the payload views borrow into it, and A: 'a bounds the result. The supervisor encodes into a spawn-scoped bumpalo bump, freed when the spawn returns, instead of assembling EncodedPayload by hand from a scope-owned BString. - The attach is safe code end to end, with no lifetime promotion anywhere: the preload ctor houses one page-backed bump in a static_cell::StaticCell, so its borrow is 'static by construction and from_env simply instantiates Client<'a> at 'static. Send + Sync assertions on EncodedPayload and Client seal the view-only design — a retained allocator handle is interior-mutable and would fail them — and the bump's own !Sync keeps its handle unstorable beyond the ctor. The redundant macOS manual Send/Sync impls on Client are deleted, and page_bump() returns the nameable PageBump so the static can be declared. One mapping serves the whole attach unless the payload outgrows the chunk, nothing comes from the global allocator, and nothing borrows the mutable process environment. - The Windows preload deserializes its payload zero-copy from the static page and forwards those original bytes to children instead of re-serializing per spawn. Co-Authored-By: Claude Fable 5 --- Cargo.lock | 14 +++ Cargo.toml | 3 +- crates/fspy/src/unix/mod.rs | 57 +++++---- crates/fspy/src/windows/mod.rs | 7 +- crates/fspy_client_unix/src/lib.rs | 38 +++--- crates/fspy_ipc_str/src/lib.rs | 12 -- crates/fspy_nostd_alloc/src/lib.rs | 31 ++++- crates/fspy_preload_unix/Cargo.toml | 1 + crates/fspy_preload_unix/src/client.rs | 15 ++- .../src/windows/client.rs | 10 +- crates/fspy_shared/src/ipc/channel/mod.rs | 77 +++++++----- crates/fspy_shared/src/windows/mod.rs | 2 +- crates/fspy_shared_unix/Cargo.toml | 1 + crates/fspy_shared_unix/src/payload.rs | 113 ++++++++++++++---- .../fspy_shared_unix/src/spawn/linux/mod.rs | 2 +- crates/fspy_shared_unix/src/spawn/macos.rs | 2 +- 16 files changed, 264 insertions(+), 121 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5896568a3..f0ff75fbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -368,6 +368,9 @@ name = "bumpalo" version = "3.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5dd9dc738b7a8311c7ade152424974d8115f2cdad61e8dab8dac9f2362298510" +dependencies = [ + "allocator-api2", +] [[package]] name = "bytemuck" @@ -1377,6 +1380,7 @@ dependencies = [ "fspy_shared_unix", "libc", "nix 0.31.2", + "static_cell", ] [[package]] @@ -1447,6 +1451,7 @@ dependencies = [ name = "fspy_shared_unix" version = "0.0.0" dependencies = [ + "allocator-api2", "anyhow", "base64", "bstr", @@ -3596,6 +3601,15 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "static_cell" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0530892bb4fa575ee0da4b86f86c667132a94b74bb72160f58ee5a4afec74c23" +dependencies = [ + "portable-atomic", +] + [[package]] name = "strsim" version = "0.11.1" diff --git a/Cargo.toml b/Cargo.toml index 3b76cb483..89e0783ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,7 +52,7 @@ bitflags = "2.10.0" brush-parser = "0.4.0" bstr = { version = "1.12.0", default-features = false } bump-scope = { version = "2", default-features = false, features = ["allocator-api2-02"] } -bumpalo = { version = "3.17.0", features = ["collections"] } +bumpalo = { version = "3.17.0", features = ["allocator-api2", "collections"] } bytemuck = { version = "1.23.0", features = ["extern_crate_alloc", "must_cast"] } cargo-platform = "=0.3.2" cc = "1.2.39" @@ -138,6 +138,7 @@ fspy_nostd = { path = "crates/fspy_nostd" } fspy_nostd_alloc = { path = "crates/fspy_nostd_alloc" } similar = "3.0.0" smallvec = { version = "2.0.0-alpha.12", features = ["std"] } +static_cell = "2" snapshot_test = { path = "crates/snapshot_test" } socket_ipc = { path = "crates/socket_ipc" } stackalloc = "1.2.1" diff --git a/crates/fspy/src/unix/mod.rs b/crates/fspy/src/unix/mod.rs index 3fb25b55e..194cdd28e 100644 --- a/crates/fspy/src/unix/mod.rs +++ b/crates/fspy/src/unix/mod.rs @@ -11,8 +11,6 @@ use fspy_seccomp_unotify::supervisor::supervise; use fspy_shared::ipc::PathAccess; #[cfg(not(target_env = "musl"))] use fspy_shared::ipc::{IpcStr, channel::channel}; -#[cfg(target_os = "macos")] -use fspy_shared_unix::payload::Artifacts; use fspy_shared_unix::{ exec::ExecResolveConfig, payload::{Payload, encode_payload}, @@ -29,9 +27,15 @@ use crate::ipc::ChannelAccesses; use crate::{ChildTermination, Command, TrackedChild, arena::PathAccessArena, error::SpawnError}; #[derive(Debug)] +#[cfg_attr( + target_os = "macos", + expect(clippy::struct_field_names, reason = "each field names a distinct injected path") +)] pub struct SpyImpl { #[cfg(target_os = "macos")] - artifacts: Artifacts, + bash_path: Box, + #[cfg(target_os = "macos")] + coreutils_path: Box, #[cfg(not(target_env = "musl"))] preload_path: Box, @@ -58,15 +62,19 @@ impl SpyImpl { #[cfg(not(target_env = "musl"))] preload_path, #[cfg(target_os = "macos")] - artifacts: { - let coreutils_path = - macos_artifacts::COREUTILS_BINARY.materialize().executable().at(dir)?; - let bash_path = macos_artifacts::OILS_BINARY.materialize().executable().at(dir)?; - Artifacts { - bash_path: bash_path.as_path().into(), - coreutils_path: coreutils_path.as_path().into(), - } - }, + bash_path: macos_artifacts::OILS_BINARY + .materialize() + .executable() + .at(dir)? + .as_path() + .into(), + #[cfg(target_os = "macos")] + coreutils_path: macos_artifacts::COREUTILS_BINARY + .materialize() + .executable() + .at(dir)? + .as_path() + .into(), }) } @@ -79,25 +87,32 @@ impl SpyImpl { let supervisor = supervise::().map_err(SpawnError::Supervisor)?; #[cfg(not(target_env = "musl"))] - let (ipc_channel_conf, ipc_receiver) = - channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global) - .map_err(SpawnError::ChannelCreation)?; + let ipc_receiver = channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global) + .map_err(SpawnError::ChannelCreation)?; let payload = Payload { #[cfg(not(target_env = "musl"))] - ipc_channel_conf, - - #[cfg(target_os = "macos")] - artifacts: self.artifacts.clone(), + ipc_channel_conf: ipc_receiver.conf(), + #[cfg(target_env = "musl")] + ipc_channel_conf: core::marker::PhantomData, #[cfg(not(target_env = "musl"))] - preload_path: self.preload_path.clone(), + preload_path: &self.preload_path, + + #[cfg(target_os = "macos")] + artifacts: fspy_shared_unix::payload::Artifacts { + bash_path: &self.bash_path, + coreutils_path: &self.coreutils_path, + }, #[cfg(target_os = "linux")] seccomp_payload: supervisor.payload().clone(), }; - let encoded_payload = encode_payload(payload); + // Spawn-scoped storage for the encoded payload, freed when this + // spawn returns. + let payload_bump = bumpalo::Bump::new(); + let encoded_payload = encode_payload(payload, &payload_bump); let mut exec = command.get_exec(); let mut exec_resolve_accesses = PathAccessArena::default(); diff --git a/crates/fspy/src/windows/mod.rs b/crates/fspy/src/windows/mod.rs index 114c796e0..096335078 100644 --- a/crates/fspy/src/windows/mod.rs +++ b/crates/fspy/src/windows/mod.rs @@ -83,9 +83,8 @@ impl SpyImpl { command.creation_flags(CREATE_SUSPENDED); - let (channel_conf, receiver) = - channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global) - .map_err(SpawnError::ChannelCreation)?; + let receiver = channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global) + .map_err(SpawnError::ChannelCreation)?; let mut spawn_success = false; let spawn_success = &mut spawn_success; @@ -105,7 +104,7 @@ impl SpyImpl { } let payload = Payload { - channel_conf: channel_conf.clone(), + channel_conf: receiver.conf(), ansi_dll_path_with_nul: ansi_dll_path_with_nul.to_bytes(), }; let payload_bytes = wincode::serialize(&payload).unwrap(); diff --git a/crates/fspy_client_unix/src/lib.rs b/crates/fspy_client_unix/src/lib.rs index 25c5c98be..2165a00cf 100644 --- a/crates/fspy_client_unix/src/lib.rs +++ b/crates/fspy_client_unix/src/lib.rs @@ -20,28 +20,36 @@ use fspy_shared_unix::{ }; use raw_exec::RawExec; -pub struct Client { - encoded_payload: EncodedPayload, +pub struct Client<'a> { + encoded_payload: EncodedPayload<'a>, ipc_sender: Option, } -// SAFETY: construction owns every field, later methods borrow them immutably, -// and the sender synchronizes its shared-memory access. -#[cfg(target_os = "macos")] -unsafe impl Sync for Client {} -// SAFETY: ownership of every field can move with the client, and the sender -// synchronizes its shared-memory access. -#[cfg(target_os = "macos")] -unsafe impl Send for Client {} +// Seals the view-only design: the client holds views of leaked memory and +// the sender, never an allocator. A retained allocator handle is +// interior-mutable and would fail this assertion. (`Sender`'s own manual +// `Send`/`Sync` impls are the one audited exception the check trusts.) +const _: () = { + const fn assert_send_sync() {} + assert_send_sync::>(); +}; -impl Debug for Client { +impl Debug for Client<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Client").finish() } } -impl Client { - /// Constructs a client from the encoded payload in the process environment. +impl<'a> Client<'a> { + /// Constructs a client from the encoded payload in the process + /// environment, leaking the payload's storage into `allocator`. + /// + /// The sender's temporary path decode also comes from `allocator` and + /// drops before this returns; a bump allocator gets that space back, + /// since the block is its most recent allocation. `A: 'a` bounds the + /// client's lifetime — a `'static` allocator yields a `Client<'static>` + /// with no further ceremony — and the client never retains the + /// allocator itself (see the `Send + Sync` assertion above). /// /// # Panics /// @@ -50,9 +58,9 @@ impl Client { /// [`ChannelConf::sender`](fspy_shared::ipc::channel::ChannelConf::sender)). pub fn from_env( envs: impl Iterator, - allocator: impl Allocator, + allocator: impl Allocator + Clone + 'a, ) -> Self { - let encoded_payload = decode_payload_from_env(envs).unwrap(); + let encoded_payload = decode_payload_from_env(envs, allocator.clone()).unwrap(); // `None` when the channel is already over, which happens when this // process starts after the root target exited. Nothing is said diff --git a/crates/fspy_ipc_str/src/lib.rs b/crates/fspy_ipc_str/src/lib.rs index 691707897..ac014bf48 100644 --- a/crates/fspy_ipc_str/src/lib.rs +++ b/crates/fspy_ipc_str/src/lib.rs @@ -94,12 +94,6 @@ impl IpcStr { Self::wrap_ref(bump.alloc_slice_copy(&self.data)) } - /// Copies this IPC string into a box. - #[must_use] - pub fn to_boxed(&self) -> Box { - Self::wrap_box(self.data.into()) - } - /// Creates an IPC string that borrows the code units of `path`, without /// its NUL terminator. /// @@ -210,12 +204,6 @@ impl<'a, S: AsRef + ?Sized> From<&'a S> for &'a IpcStr { } } -impl Clone for Box { - fn clone(&self) -> Self { - IpcStr::wrap_box(self.data.into()) - } -} - impl> From for Box { #[cfg(unix)] fn from(value: S) -> Self { diff --git a/crates/fspy_nostd_alloc/src/lib.rs b/crates/fspy_nostd_alloc/src/lib.rs index 868de9818..fd2f9f6e4 100644 --- a/crates/fspy_nostd_alloc/src/lib.rs +++ b/crates/fspy_nostd_alloc/src/lib.rs @@ -9,7 +9,8 @@ //! fixed-size chunks; and `bump_scope::Bump`s on top. [`pooled_bump`] //! creates a bump that draws its chunks from the pool and returns them on //! drop, so frequent short-lived bumps reuse memory instead of paying two -//! syscalls each. +//! syscalls each. [`page_bump`] bypasses the pool for bumps whose chunks +//! must never be recycled into other bumps. #![cfg_attr(not(test), no_std)] @@ -34,10 +35,10 @@ use bump_scope::{ }; pub use c_string::{CString, OsCString}; #[cfg(unix)] -pub(crate) use mmap::MmapAllocator as PageAllocator; +pub use mmap::MmapAllocator as PageAllocator; use pool::ChunkPool; #[cfg(windows)] -pub(crate) use virtual_alloc::VirtualAllocator as PageAllocator; +pub use virtual_alloc::VirtualAllocator as PageAllocator; /// Every cached chunk is 64 KiB: a whole multiple of the page size on all /// supported targets, and big enough that most intercepted calls fit their @@ -87,6 +88,30 @@ impl Default for &'static ChunkPool, PageBumpSettings>; + +/// [`PageBump`]'s settings: start without a chunk, so creating one +/// allocates nothing. +pub type PageBumpSettings = ::WithGuaranteedAllocated; + +/// Creates an empty [`PageBump`]. +/// +/// Creating it allocates nothing; the first allocation maps one chunk, and +/// further chunks are mapped only if the data outgrows it. Dropping the +/// bump frees its chunks; leaking it instead makes its allocations +/// permanent. +#[must_use] +pub const fn page_bump() -> PageBump { + Bump::unallocated() +} + /// Creates a fresh bump backed by the process-wide chunk pool. /// /// Creating the bump allocates nothing; the first allocation grabs a whole diff --git a/crates/fspy_preload_unix/Cargo.toml b/crates/fspy_preload_unix/Cargo.toml index fff45719e..b6bce4661 100644 --- a/crates/fspy_preload_unix/Cargo.toml +++ b/crates/fspy_preload_unix/Cargo.toml @@ -16,6 +16,7 @@ libc = { workspace = true } nix = { workspace = true, features = ["signal", "fs", "socket", "mman", "time"] } fspy_nostd = { workspace = true } fspy_nostd_alloc = { workspace = true } +static_cell = { workspace = true } [lints] workspace = true diff --git a/crates/fspy_preload_unix/src/client.rs b/crates/fspy_preload_unix/src/client.rs index a72800974..e81a55a2d 100644 --- a/crates/fspy_preload_unix/src/client.rs +++ b/crates/fspy_preload_unix/src/client.rs @@ -3,7 +3,7 @@ use std::{cell::Cell, sync::OnceLock}; use convert::{ToAbsolutePath, ToAccessMode}; pub use fspy_client_unix::{Client, convert, raw_exec}; -static CLIENT: OnceLock = OnceLock::new(); +static CLIENT: OnceLock> = OnceLock::new(); // Resolving and reporting a file access can call another interposed function. // Suppress same-thread re-entry to prevent recursive access handling while @@ -19,7 +19,7 @@ impl Drop for ResetHandling<'_> { } } -pub fn global_client() -> Option<&'static Client> { +pub fn global_client() -> Option<&'static Client<'static>> { CLIENT.get() } @@ -45,5 +45,14 @@ fn init_client() { // SAFETY: the ctor only reads the process environment while constructing // the client and does not retain borrowed environment views. let current = unsafe { fspy_nostd::env::current() }.unwrap(); - CLIENT.set(Client::from_env(current.envs(), fspy_nostd_alloc::pooled_bump())).unwrap(); + // The attach's storage: one page-backed bump housed in a static, so + // its borrow is 'static by construction and the client comes out as + // Client<'static> with no lifetime promotion anywhere. The bump is not + // Sync, so this handle cannot be stored globally by any safe code, and + // the Send/Sync assertion on Client proves the client keeps no handle. + static BUMP: static_cell::StaticCell = + static_cell::StaticCell::new(); + let bump: &'static fspy_nostd_alloc::PageBump = BUMP.init(fspy_nostd_alloc::page_bump()); + let client = Client::from_env(current.envs(), bump); + CLIENT.set(client).unwrap(); } diff --git a/crates/fspy_preload_windows/src/windows/client.rs b/crates/fspy_preload_windows/src/windows/client.rs index 29a9771fa..7dde59c76 100644 --- a/crates/fspy_preload_windows/src/windows/client.rs +++ b/crates/fspy_preload_windows/src/windows/client.rs @@ -10,6 +10,7 @@ use winapi::{shared::minwindef::BOOL, um::winnt::HANDLE}; pub struct Client<'a> { payload: Payload<'a>, + payload_bytes: &'a [u8], ipc_sender: Option, } @@ -23,7 +24,7 @@ impl<'a> Client<'a> { // corrupts whatever that process is printing. let ipc_sender = payload.channel_conf.sender(allocator); - Self { payload, ipc_sender } + Self { payload, payload_bytes, ipc_sender } } pub fn send(&self, access: PathAccess<'_>) { @@ -36,14 +37,15 @@ impl<'a> Client<'a> { } pub unsafe fn prepare_child_process(&self, child_handle: HANDLE) -> BOOL { - let payload_bytes = wincode::serialize(&self.payload).unwrap(); + // The payload propagates to children unchanged, so forward the bytes + // this process was given instead of re-serializing. // SAFETY: FFI call to DetourCopyPayloadToProcess with valid handle and payload buffer unsafe { DetourCopyPayloadToProcess( child_handle, &PAYLOAD_ID, - payload_bytes.as_ptr().cast(), - payload_bytes.len().try_into().unwrap(), + self.payload_bytes.as_ptr().cast(), + self.payload_bytes.len().try_into().unwrap(), ) } } diff --git a/crates/fspy_shared/src/ipc/channel/mod.rs b/crates/fspy_shared/src/ipc/channel/mod.rs index d4541e300..97b3ebc5e 100644 --- a/crates/fspy_shared/src/ipc/channel/mod.rs +++ b/crates/fspy_shared/src/ipc/channel/mod.rs @@ -41,17 +41,20 @@ const SHM_BACKING_PREFIX: &str = "vite-task-fspy-"; const SLOTS: usize = 1 << 26; /// Serializable configuration to create channel senders. -#[derive(SchemaWrite, SchemaRead, Clone, Debug)] -pub struct ChannelConf { - shm_id: Box, +/// +/// A conf is a view: it borrows the path the [`Receiver`] owns (or, in a +/// receiving process, the payload bytes it was deserialized from), so +/// materializing and serializing one allocates nothing. +#[derive(SchemaWrite, SchemaRead, Clone, Copy, Debug)] +pub struct ChannelConf<'a> { + shm_id: &'a IpcStr, } -/// Creates a mpsc IPC channel with one receiver and a `ChannelConf` that can be passed around processes and used to create multiple senders. +/// Creates a mpsc IPC channel and returns its receiver. [`Receiver::conf`] +/// derives the serializable configuration that other processes use to create +/// senders. #[expect(clippy::missing_errors_doc, reason = "non-vt crate: cannot use vt_str/vt_path types")] -pub fn channel( - capacity: usize, - allocator: A, -) -> io::Result<(ChannelConf, Receiver)> { +pub fn channel(capacity: usize, allocator: A) -> io::Result> { let shm_c_path = os_c_string(shm_backing_path()?.as_os_str(), allocator)?; let handle = fspy_shm::create(shm_c_path.as_c_str().as_thin(), capacity).map_err(shm_error_to_io)?; @@ -71,9 +74,7 @@ pub fn channel( )); } - let conf = ChannelConf { shm_id: IpcStr::from_os_c_str(keeper.path.as_c_str()).to_boxed() }; - - Ok((conf, Receiver { _keeper: keeper, mapping })) + Ok(Receiver { keeper, mapping }) } /// Encodes `path` as an owned NUL-terminated platform C string. @@ -161,7 +162,7 @@ impl Drop for ShmKeeper { } } -impl ChannelConf { +impl ChannelConf<'_> { /// Creates a sender, or `None` when the channel is already over. /// /// Never blocks. `None` means the receiver removed the backing file, @@ -179,7 +180,8 @@ impl ChannelConf { #[must_use] pub fn sender(&self, allocator: A) -> Option { // The allocation is transient: the decoded path only has to outlive - // the open call below. + // the open call below, and dropping it hands the space back to a + // bump allocator, whose most recent allocation it is. let shm_path = self .shm_id .to_os_c_string_in(allocator) @@ -260,7 +262,7 @@ unsafe impl Sync for Sender {} pub struct Receiver { /// Keeps the shared memory's backing file alive for as long as senders /// may attach. - _keeper: ShmKeeper, + keeper: ShmKeeper, mapping: Mapping, } @@ -274,6 +276,13 @@ unsafe impl Send for Receiver {} unsafe impl Sync for Receiver {} impl Receiver { + /// Returns the serializable configuration other processes pass to + /// [`ChannelConf::sender`], borrowing this receiver's storage. + #[must_use] + pub fn conf(&self) -> ChannelConf<'_> { + ChannelConf { shm_id: IpcStr::from_os_c_str(self.keeper.path.as_c_str()) } + } + /// Closes the channel and returns every committed frame, borrowed from /// the shared mapping that moves into the returned [`FrameReader`]. /// @@ -298,7 +307,7 @@ impl Receiver { /// When the region cannot hold the protocol, which [`channel`] proved /// it could before any sender saw it. pub fn close(self) -> Result { - let Self { _keeper: keeper, mapping } = self; + let Self { keeper, mapping } = self; // SAFETY: `mapping` was created zero-initialized by `channel`, its // address is stable and independently owned, and all attached // processes access it only through the `shm_io` protocol. @@ -366,11 +375,13 @@ mod tests { /// must still attach. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn sender_ignores_changed_temp_and_working_directory() { - let (conf, receiver) = channel(CAPACITY, Global).unwrap(); + let receiver = channel(CAPACITY, Global).unwrap(); + let conf = wincode::serialize(&receiver.conf()).unwrap(); let changed_cwd = temp_dir().join(format!("fspy-ipc-changed-cwd-{}", Uuid::new_v4())); fs::create_dir(&changed_cwd).unwrap(); - let mut command = command_for_fn!(conf, |conf: ChannelConf| { + let mut command = command_for_fn!(conf, |conf: Vec| { + let conf: ChannelConf = wincode::deserialize(&conf).unwrap(); let sender = conf.sender(Global).unwrap(); let frame_size = NonZeroUsize::new(2).unwrap(); let mut frame = sender.writer.claim_frame(frame_size).unwrap(); @@ -396,8 +407,8 @@ mod tests { /// here rather than in a build. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn sender_round_trips_records() { - let (conf, receiver) = channel(CAPACITY, Global).unwrap(); - let sender = conf.sender(Global).unwrap(); + let receiver = channel(CAPACITY, Global).unwrap(); + let sender = receiver.conf().sender(Global).unwrap(); // A record path carries the platform's own string form: bytes on // unix, UTF-16 on Windows. #[cfg(unix)] @@ -427,8 +438,10 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn smoke() { - let (conf, receiver) = channel(CAPACITY, Global).unwrap(); - let cmd = command_for_fn!(conf, |conf: ChannelConf| { + let receiver = channel(CAPACITY, Global).unwrap(); + let conf = wincode::serialize(&receiver.conf()).unwrap(); + let cmd = command_for_fn!(conf, |conf: Vec| { + let conf: ChannelConf = wincode::deserialize(&conf).unwrap(); let sender = conf.sender(Global).unwrap(); let frame_size = NonZeroUsize::new(2).unwrap(); let mut frame = sender.writer.claim_frame(frame_size).unwrap(); @@ -449,10 +462,12 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[expect(clippy::print_stdout, reason = "test diagnostics")] async fn forbid_new_senders_after_close() { - let (conf, receiver) = channel(CAPACITY, Global).unwrap(); + let receiver = channel(CAPACITY, Global).unwrap(); + let conf = wincode::serialize(&receiver.conf()).unwrap(); let _frames = receiver.close().unwrap(); - let cmd = command_for_fn!(conf, |conf: ChannelConf| { + let cmd = command_for_fn!(conf, |conf: Vec| { + let conf: ChannelConf = wincode::deserialize(&conf).unwrap(); print!("{}", conf.sender(Global).is_some()); }); let output = std::process::Command::from(cmd).output().unwrap(); @@ -462,10 +477,12 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[expect(clippy::print_stdout, reason = "test diagnostics")] async fn forbid_new_senders_after_receiver_dropped() { - let (conf, receiver) = channel(CAPACITY, Global).unwrap(); + let receiver = channel(CAPACITY, Global).unwrap(); + let conf = wincode::serialize(&receiver.conf()).unwrap(); drop(receiver); - let cmd = command_for_fn!(conf, |conf: ChannelConf| { + let cmd = command_for_fn!(conf, |conf: Vec| { + let conf: ChannelConf = wincode::deserialize(&conf).unwrap(); print!("{}", conf.sender(Global).is_some()); }); let output = std::process::Command::from(cmd).output().unwrap(); @@ -476,8 +493,8 @@ mod tests { /// claim any new frame afterwards. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn attached_sender_cannot_claim_after_close() { - let (conf, receiver) = channel(CAPACITY, Global).unwrap(); - let sender = conf.sender(Global).unwrap(); + let receiver = channel(CAPACITY, Global).unwrap(); + let sender = receiver.conf().sender(Global).unwrap(); let mut frame = sender.writer.claim_frame(NonZeroUsize::new(2).unwrap()).unwrap(); frame.copy_from_slice(&[4, 2]); @@ -494,9 +511,11 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn concurrent_senders() { - let (conf, receiver) = channel(CAPACITY, Global).unwrap(); + let receiver = channel(CAPACITY, Global).unwrap(); + let conf = wincode::serialize(&receiver.conf()).unwrap(); for i in 0u16..200 { - let cmd = command_for_fn!((conf.clone(), i), |(conf, i): (ChannelConf, u16)| { + let cmd = command_for_fn!((conf.clone(), i), |(conf, i): (Vec, u16)| { + let conf: ChannelConf = wincode::deserialize(&conf).unwrap(); let sender = conf.sender(Global).unwrap(); let data_to_send = i.to_string(); let mut frame = sender diff --git a/crates/fspy_shared/src/windows/mod.rs b/crates/fspy_shared/src/windows/mod.rs index cf7c536be..b2700c122 100644 --- a/crates/fspy_shared/src/windows/mod.rs +++ b/crates/fspy_shared/src/windows/mod.rs @@ -22,6 +22,6 @@ DEFINE_GUID!( #[derive(SchemaWrite, SchemaRead, Debug, Clone)] pub struct Payload<'a> { - pub channel_conf: ChannelConf, + pub channel_conf: ChannelConf<'a>, pub ansi_dll_path_with_nul: &'a [u8], } diff --git a/crates/fspy_shared_unix/Cargo.toml b/crates/fspy_shared_unix/Cargo.toml index 9cf964474..7ae6990b0 100644 --- a/crates/fspy_shared_unix/Cargo.toml +++ b/crates/fspy_shared_unix/Cargo.toml @@ -6,6 +6,7 @@ license.workspace = true publish = false [target.'cfg(unix)'.dependencies] +allocator-api2 = { workspace = true, features = ["alloc"] } anyhow = { workspace = true } base64 = { workspace = true } wincode = { workspace = true, features = ["derive"] } diff --git a/crates/fspy_shared_unix/src/payload.rs b/crates/fspy_shared_unix/src/payload.rs index 14f30229a..8276ba5ee 100644 --- a/crates/fspy_shared_unix/src/payload.rs +++ b/crates/fspy_shared_unix/src/payload.rs @@ -1,21 +1,31 @@ +use allocator_api2::alloc::Allocator; use base64::{Engine as _, prelude::BASE64_STANDARD_NO_PAD}; -use bstr::BString; +use bstr::BStr; #[cfg(not(target_env = "musl"))] use fspy_shared::ipc::IpcStr; #[cfg(not(target_env = "musl"))] use fspy_shared::ipc::channel::ChannelConf; use wincode::{SchemaRead, SchemaWrite}; +/// The payload as it travels between processes. +/// +/// A payload is a view: every path in it borrows from storage its producer +/// owns — the supervisor's session state, or the storage a +/// [`decode_payload_from_env`] caller supplies — so serializing one, and +/// deserializing one in the preload, allocates nothing for the borrowed +/// fields. #[derive(Debug, SchemaWrite, SchemaRead)] -pub struct Payload { +pub struct Payload<'a> { #[cfg(not(target_env = "musl"))] - pub ipc_channel_conf: ChannelConf, + pub ipc_channel_conf: ChannelConf<'a>, + #[cfg(target_env = "musl")] + pub ipc_channel_conf: core::marker::PhantomData<&'a ()>, #[cfg(not(target_env = "musl"))] - pub preload_path: Box, + pub preload_path: &'a IpcStr, #[cfg(target_os = "macos")] - pub artifacts: Artifacts, + pub artifacts: Artifacts<'a>, #[cfg(target_os = "linux")] #[cfg_attr( @@ -26,43 +36,93 @@ pub struct Payload { } #[cfg(target_os = "macos")] -#[derive(Debug, SchemaWrite, SchemaRead, Clone)] -pub struct Artifacts { - pub bash_path: Box, - pub coreutils_path: Box, +#[derive(Debug, SchemaWrite, SchemaRead, Clone, Copy)] +pub struct Artifacts<'a> { + pub bash_path: &'a IpcStr, + pub coreutils_path: &'a IpcStr, } pub(crate) const PAYLOAD_ENV_NAME: &str = "FSPY_PAYLOAD"; -pub struct EncodedPayload { - pub payload: Payload, - pub encoded_string: BString, +/// A payload together with its encoded form, for handing to child processes. +/// +/// Like [`Payload`], this is strictly a view: it lives as long as the +/// storage [`decode_payload_from_env`] allocated into. +pub struct EncodedPayload<'a> { + pub payload: Payload<'a>, + pub encoded_string: &'a BStr, } -/// Encodes the fspy payload into a base64 string for transmission via environment variable +// Seals the view-only design: every field is an inert borrow of leaked +// bytes. A retained allocator handle is interior-mutable and would fail +// this assertion. +const _: () = { + const fn assert_send_sync() {} + assert_send_sync::>(); +}; + +/// Encodes the payload into its base64 environment form and assembles the +/// [`EncodedPayload`] that travels to child processes. +/// +/// The serialization buffer is transient; the encoded string is leaked +/// into `allocator`, and `A: 'a` bounds the result's lifetime just as in +/// [`decode_payload_from_env`]. /// /// # Panics /// -/// Panics if serialization fails, which should never happen for valid `Payload` structs. +/// Panics if serialization fails, which should never happen for valid +/// [`Payload`] structs. #[must_use] -pub fn encode_payload(payload: Payload) -> EncodedPayload { - let bytes = wincode::serialize(&payload).unwrap(); - let encoded_string = BASE64_STANDARD_NO_PAD.encode(&bytes); - EncodedPayload { payload, encoded_string: encoded_string.into() } +pub fn encode_payload<'a, A: Allocator + Clone + 'a>( + payload: Payload<'a>, + allocator: A, +) -> EncodedPayload<'a> { + let serialized_size = usize::try_from(wincode::serialized_size(&payload).unwrap()) + .expect("serialized size exceeds usize"); + let mut buffer = allocator_api2::vec::Vec::with_capacity_in(serialized_size, allocator.clone()); + buffer.resize(serialized_size, 0); + let mut writer: &mut [u8] = &mut buffer; + wincode::serialize_into(&mut writer, &payload).unwrap(); + assert!(writer.is_empty(), "the payload wrote fewer bytes than the size it reported"); + + let encoded_len = + base64::encoded_len(serialized_size, false).expect("encoded payload length exceeds usize"); + let mut encoded = allocator_api2::vec::Vec::with_capacity_in(encoded_len, allocator); + encoded.resize(encoded_len, 0); + let written = BASE64_STANDARD_NO_PAD.encode_slice(&buffer, &mut encoded).unwrap(); + encoded.truncate(written); + EncodedPayload { payload, encoded_string: BStr::new(encoded.leak()) } } /// Decodes the fspy payload from an iterator over environment entries. /// +/// The returned payload borrows allocations that are leaked into +/// `allocator` and never individually freed. `A: 'a` bounds the payload's +/// lifetime: a borrowed bump yields a payload that dies with the borrow, +/// and a `'static` allocator yields a `'static` payload outright. +/// +/// The payload never borrows the process environment itself: the +/// environment is not stable storage (any `setenv` may move or rewrite it), +/// so the encoded value is copied out before it is used. +/// /// # Errors /// /// Returns an error if the payload environment variable is missing, base64 /// decoding fails, or deserialization fails. -pub fn decode_payload_from_env( +pub fn decode_payload_from_env<'a, A: Allocator + Clone + 'a>( mut envs: impl Iterator, -) -> anyhow::Result { + allocator: A, +) -> anyhow::Result> { let Some(encoded_string) = envs.find_map(|(name, value)| { if AsRef::<[u8]>::as_ref(name) == PAYLOAD_ENV_NAME.as_bytes() { - value.map(|value| BString::from(value.as_units())) + value.map(|value| { + let mut encoded = allocator_api2::vec::Vec::with_capacity_in( + value.as_units().len(), + allocator.clone(), + ); + encoded.extend_from_slice(value.as_units()); + BStr::new(encoded.leak()) + }) } else { None } @@ -70,11 +130,12 @@ pub fn decode_payload_from_env( anyhow::bail!("Environment variable '{PAYLOAD_ENV_NAME}' not found"); }; - decode_payload(encoded_string) -} + let decoded_len_estimate = base64::decoded_len_estimate(encoded_string.len()); + let mut buffer = allocator_api2::vec::Vec::with_capacity_in(decoded_len_estimate, allocator); + buffer.resize(decoded_len_estimate, 0); + let decoded_len = BASE64_STANDARD_NO_PAD.decode_slice(encoded_string, &mut buffer)?; + buffer.truncate(decoded_len); + let payload: Payload<'a> = wincode::deserialize_exact(buffer.leak())?; -fn decode_payload(encoded_string: BString) -> anyhow::Result { - let bytes = BASE64_STANDARD_NO_PAD.decode(&encoded_string)?; - let payload: Payload = wincode::deserialize_exact(&bytes)?; Ok(EncodedPayload { payload, encoded_string }) } diff --git a/crates/fspy_shared_unix/src/spawn/linux/mod.rs b/crates/fspy_shared_unix/src/spawn/linux/mod.rs index d3197da00..1bac0529e 100644 --- a/crates/fspy_shared_unix/src/spawn/linux/mod.rs +++ b/crates/fspy_shared_unix/src/spawn/linux/mod.rs @@ -53,7 +53,7 @@ pub fn handle_exec( LD_PRELOAD, encoded_payload.payload.preload_path.as_os_str().as_bytes(), ); - ensure_env(&mut command.envs, PAYLOAD_ENV_NAME, &encoded_payload.encoded_string)?; + ensure_env(&mut command.envs, PAYLOAD_ENV_NAME, encoded_payload.encoded_string)?; return Ok(None); } } diff --git a/crates/fspy_shared_unix/src/spawn/macos.rs b/crates/fspy_shared_unix/src/spawn/macos.rs index 88bd7d0a4..eeab53e41 100644 --- a/crates/fspy_shared_unix/src/spawn/macos.rs +++ b/crates/fspy_shared_unix/src/spawn/macos.rs @@ -69,7 +69,7 @@ pub fn handle_exec( DYLD_INSERT_LIBRARIES, encoded_payload.payload.preload_path.as_os_str().as_bytes(), ); - ensure_env(&mut command.envs, PAYLOAD_ENV_NAME, &encoded_payload.encoded_string)?; + ensure_env(&mut command.envs, PAYLOAD_ENV_NAME, encoded_payload.encoded_string)?; } else { command.envs.retain(|(name, _)| { name != DYLD_INSERT_LIBRARIES && name != PAYLOAD_ENV_NAME.as_bytes()