diff --git a/Cargo.lock b/Cargo.lock index be7e52c36..e3071154d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1424,6 +1424,7 @@ dependencies = [ "ctor", "fspy_ipc_str", "fspy_shm", + "omnipath", "rustc-hash", "subprocess_test", "thiserror 2.0.18", @@ -1459,7 +1460,6 @@ version = "0.0.0" dependencies = [ "ctor", "fspy_nostd", - "omnipath", "subprocess_test", "uuid", "windows-sys 0.61.2", diff --git a/crates/fspy_shared/Cargo.toml b/crates/fspy_shared/Cargo.toml index f81449318..f5ed6c105 100644 --- a/crates/fspy_shared/Cargo.toml +++ b/crates/fspy_shared/Cargo.toml @@ -20,6 +20,7 @@ vt_path = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] bytemuck = { workspace = true } +omnipath = { workspace = true } winapi = { workspace = true, features = ["std"] } [dev-dependencies] diff --git a/crates/fspy_shared/src/ipc/channel/mod.rs b/crates/fspy_shared/src/ipc/channel/mod.rs index 2bbaa4fa3..396fbc620 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, ops::Deref, path::PathBuf}; -use fspy_shm::{Mapping, ShmKeeper}; +use fspy_shm::Mapping; pub use shm_io::FrameMut; use shm_io::{ShmReader, ShmWriter}; use tracing::debug; @@ -13,6 +13,14 @@ use wincode::{SchemaRead, SchemaWrite}; use super::IpcStr; +/// Prefix of shared-memory backing file names inside the system temporary +/// directory. +/// +/// The files sit directly in the temporary directory. A shared subdirectory +/// would belong to whichever user created it first and block everyone else; +/// uniquely named `0o600` files in the sticky-bit temp directory avoid that. +const SHM_BACKING_PREFIX: &str = "vite-task-fspy-"; + /// Serializable configuration to create channel senders. #[derive(SchemaWrite, SchemaRead, Clone, Debug)] pub struct ChannelConf { @@ -26,18 +34,68 @@ 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())); - let (keeper, handle) = fspy_shm::create(capacity)?; + let shm_path = shm_backing_path()?; + let handle = fspy_shm::create(shm_path.as_os_str(), capacity)?; + // The keeper exists from here on, so every error path below cleans up. + let keeper = ShmKeeper { path: shm_path }; let mapping = handle.map()?; let conf = ChannelConf { lock_file_path: lock_file_path.as_os_str().into(), - shm_id: keeper.id().into(), + shm_id: keeper.path.as_os_str().into(), }; let receiver = Receiver::new(lock_file_path, keeper, mapping)?; Ok((conf, receiver)) } +/// Returns a fresh absolute path for a shared-memory backing file. +fn shm_backing_path() -> io::Result { + // `temp_dir` reflects `TMPDIR` verbatim, which may be relative. The path + // travels to processes with other working directories, so resolve it + // against the creator's current directory first. + let path = std::path::absolute(temp_dir())? + .join(format!("{SHM_BACKING_PREFIX}{}.shm", Uuid::new_v4().simple())); + #[cfg(windows)] + let path = to_verbatim_if_long(path)?; + Ok(path) +} + +/// Converts long paths to verbatim (`\\?\`) form up front, so every later use +/// of the path — creation here, opening in any process, removal — stays clear +/// of the legacy `MAX_PATH` limit without relying on the system's long-path +/// opt-in, which the arbitrary processes opening shared memory could not +/// count on anyway. +#[cfg(windows)] +fn to_verbatim_if_long(path: PathBuf) -> io::Result { + use std::os::windows::ffi::OsStrExt as _; + + use omnipath::windows::WinPathExt as _; + + // The length at which std's own Windows path conversion switches to a + // verbatim path. + const VERBATIM_THRESHOLD: usize = 248; + + if path.as_os_str().encode_wide().count() >= VERBATIM_THRESHOLD { + return path.to_verbatim(); + } + Ok(path) +} + +/// Keeps the shared memory's backing path alive and removes it on drop. +/// +/// Removal is cleanup, not a stop signal: later opens fail, but existing +/// handles and mappings keep reading and writing; see [`fspy_shm::remove`]. +struct ShmKeeper { + path: PathBuf, +} + +impl Drop for ShmKeeper { + fn drop(&mut self) { + let _ = fspy_shm::remove(self.path.as_os_str()); + } +} + impl ChannelConf { /// Creates a sender. /// @@ -93,7 +151,8 @@ unsafe impl Sync for Sender {} pub struct Receiver { lock_file_path: PathBuf, lock_file: File, - /// Keeps the backing file's name alive for as long as senders may attach. + /// Keeps the shared memory's backing file alive for as long as senders + /// may attach. _keeper: ShmKeeper, mapping: Mapping, } @@ -156,13 +215,40 @@ impl<'a> Deref for ReceiverLockGuard<'a> { #[cfg(test)] mod tests { - use std::{num::NonZeroUsize, str::from_utf8}; + use std::{ffi::OsString, fs, num::NonZeroUsize, str::from_utf8}; use bstr::B; use subprocess_test::command_for_fn; use super::*; + /// The shared-memory path is generated absolute, so a sender in a process + /// with a different working directory and a relative temporary directory + /// must still attach. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn sender_ignores_changed_temp_and_working_directory() { + let (conf, receiver) = channel(100).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 sender = conf.sender().unwrap(); + let frame_size = NonZeroUsize::new(2).unwrap(); + let mut frame = sender.claim_frame(frame_size).unwrap(); + frame.copy_from_slice(&[4, 2]); + }); + command.cwd = changed_cwd.clone(); + for name in ["TMPDIR", "TMP", "TEMP"] { + command.envs.insert(OsString::from(name), OsString::from("changed-relative-tmp")); + } + let succeeded = std::process::Command::from(command).status().unwrap().success(); + fs::remove_dir(changed_cwd).unwrap(); + assert!(succeeded); + + let lock = receiver.lock().unwrap(); + assert_eq!(lock.iter_frames().next().unwrap(), &[4, 2]); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn smoke() { let (conf, receiver) = channel(100).unwrap(); diff --git a/crates/fspy_shared/src/ipc/channel/shm_io.rs b/crates/fspy_shared/src/ipc/channel/shm_io.rs index 4b4585a90..59ebe83e8 100644 --- a/crates/fspy_shared/src/ipc/channel/shm_io.rs +++ b/crates/fspy_shared/src/ipc/channel/shm_io.rs @@ -672,8 +672,10 @@ mod tests { const SHM_SIZE: usize = 1024 * 1024; - let (keeper, handle) = fspy_shm::create(SHM_SIZE).unwrap(); - let shm_name = keeper.id().to_str().expect("test temp dir is UTF-8").to_owned(); + let shm_path = crate::ipc::channel::shm_backing_path().unwrap(); + let handle = fspy_shm::create(shm_path.as_os_str(), SHM_SIZE).unwrap(); + let _keeper = crate::ipc::channel::ShmKeeper { path: shm_path.clone() }; + let shm_name = shm_path.to_str().expect("test temp dir is UTF-8").to_owned(); // Map before the children run. Windows keeps views coherent while they // exist at the same time; a view created after every writer exited can // observe the file before the writers' dirty pages reach it. diff --git a/crates/fspy_shm/Cargo.toml b/crates/fspy_shm/Cargo.toml index e53ea56b0..ad38ff3d5 100644 --- a/crates/fspy_shm/Cargo.toml +++ b/crates/fspy_shm/Cargo.toml @@ -7,14 +7,10 @@ license.workspace = true publish = false rust-version.workspace = true -[dependencies] -uuid = { workspace = true, features = ["v4"] } - [target.'cfg(any(unix, windows))'.dependencies] fspy_nostd = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] -omnipath = { workspace = true } windows-sys = { workspace = true, features = [ "Win32_Foundation", "Win32_Storage_FileSystem", @@ -25,6 +21,7 @@ windows-sys = { workspace = true, features = [ [dev-dependencies] ctor = { workspace = true } subprocess_test = { workspace = true } +uuid = { workspace = true, features = ["v4"] } [lints] workspace = true diff --git a/crates/fspy_shm/README.md b/crates/fspy_shm/README.md index 6be9c9011..c3113abe1 100644 --- a/crates/fspy_shm/README.md +++ b/crates/fspy_shm/README.md @@ -1,8 +1,8 @@ # `fspy_shm` -`fspy_shm` is the private shared-memory layer used by fspy IPC channels. It gives the channel one API for creating a mapping, passing its identifier to another process, and opening additional views of the same bytes. +`fspy_shm` is the private shared-memory layer used by fspy IPC channels. It gives the channel one API for creating a mapping at a caller-chosen path, opening additional views of the same bytes from any process that knows the path, and removing the backing file. -`fspy_shm` exposes only the operations used by fspy. Treat an identifier as an opaque `OsStr`; do not depend on how it is built. +`fspy_shm` exposes only the operations used by fspy. The caller owns the path: it decides where the backing file lives, passes the same path to every process that opens the shared memory, and removes it when the shared memory is no longer needed. The fspy channel generates an absolute uniquely-named path in the system temporary directory and holds it in a keeper that removes it on drop. ## API @@ -10,36 +10,35 @@ The public API is defined in [`src/lib.rs`](src/lib.rs). | API | Contract | | --------------------- | --------------------------------------------------------------------------------------- | -| `create(size)` | Creates a zero-initialized backing file and returns its `ShmKeeper` and an `ShmHandle`. | -| `open(id)` | Opens an `ShmHandle` on the shared memory identified by `id`. | -| `ShmKeeper::id()` | Returns the identifier another process passes to `open`. | +| `create(path, size)` | Creates a zero-initialized backing file at `path` and returns an opened `ShmHandle`. | +| `open(path)` | Opens an `ShmHandle` on the shared memory backed by the file at `path`. | +| `remove(path)` | Removes the backing file. Later opens fail; existing handles and mappings keep working. | | `ShmHandle::map()` | Maps the shared bytes. Callable more than once. | | `Mapping::len()` | Returns the mapped size. | | `Mapping::as_ptr()` | Returns a mutable raw pointer to the first byte. | | `Mapping::as_slice()` | Returns the bytes as a shared slice. The caller must prevent mutation for its lifetime. | -`ShmKeeper` is the name: while it lives, `open` succeeds, and dropping it removes the backing 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. `Mapping` is the bytes: it keeps them alive until dropped and can do nothing else. None of the three synchronizes memory access. The fspy channel adds that on top with atomic frame headers and a lock file: senders hold a shared file lock while writing, and the receiver takes the exclusive lock before reading, which waits for existing senders and rejects new ones. +`ShmHandle` is the opened file: `create` returns one so the creator never looks its own file up by path, and `open` returns one to everybody else. `Mapping` is the bytes: it keeps them alive until dropped and can do nothing else. Neither synchronizes memory access. The fspy channel adds that on top with atomic frame headers and a lock file: senders hold a shared file lock while writing, and the receiver takes the exclusive lock before reading, which waits for existing senders and rejects new ones. Every byte in a mapping returned by `create` is initially zero. `open` exposes the mapping's current contents and does not reinitialize them. ## Implementation -One implementation serves every platform: a sparse file named `vite-task-fspy-.shm` directly in the system temporary directory. The identifier is the file's absolute path, so another process opens the mapping by opening that path. There is no broker, no global object name, and no asynchronous runtime. The files sit in the temporary directory itself rather than a shared subdirectory: a subdirectory would belong to whichever user created it first and block everyone else, while uniquely named `0o600` files in a sticky-bit directory work for all users. +One implementation serves every platform: a sparse file at the caller's path. Another process opens the mapping by opening that path. There is no broker, no global object name, and no asynchronous runtime. Only written pages ever occupy memory or disk. The multi-gigabyte capacity fspy asks for therefore costs about as much as the data a run actually records. -Mapping goes through `memmap2` on every platform. The remaining platform-specific parts are three short passages: +Every operation goes through [`fspy_nostd`](../fspy_nostd) wrappers or direct Win32 calls. The platform-specific parts are three short passages: -| Concern | Unix | Windows | -| ----------------- | ---------------------------------------- | --------------------------------------------------------------------------- | -| Same-user access | `mode(0o600)` on the backing file | the per-user `%TEMP%` ACL | -| Sparseness | file holes, produced by setting a length | `FSCTL_SET_SPARSE` before setting a length, or NTFS allocates every cluster | -| Keeper cleanup | unlink the path | unlink the path; see the fallback below | -| Descriptor safety | `O_CLOEXEC`, the Rust standard default | non-inheritable handles, the Rust standard default | +| Concern | Unix | Windows | +| ---------------- | ---------------------------------------- | --------------------------------------------------------------------------- | +| Same-user access | `mode(0o600)` on the backing file | the per-user `%TEMP%` ACL of the caller's chosen directory | +| Sparseness | file holes, produced by setting a length | `FSCTL_SET_SPARSE` before setting a length, or NTFS allocates every cluster | +| Removal | unlink the path | POSIX delete via `FileDispositionInfoEx`; see below | `FILE_ATTRIBUTE_TEMPORARY` asks Windows to keep the data in memory when it can. Creation fails on a volume without sparse-file support. -The keeper removes the name with `remove_file` on every platform. Modern Windows deletes with POSIX semantics: the name goes away at once, while [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) until the last one goes away. The first page also reserves the right to fail the delete while a mapped view exists, and Windows versions without POSIX delete do fail it. The keeper then falls back to reopening the file with `FILE_FLAG_DELETE_ON_CLOSE` and closing it, which deletes the file once every handle to it is closed. +`remove` unlinks the path on Unix. On Windows it relies on [POSIX delete semantics](https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/ns-ntddk-_file_disposition_information_ex), which requires NTFS on Windows 10 1607 or newer: the name goes away at once, while existing handles keep working and [mapped views keep the data alive](https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-createfilemappingw) until the last one goes away. ## Options considered @@ -61,12 +60,10 @@ Earlier revisions rejected temporary files because dirty pages can reach disk. O ## Lifetime semantics -`create` returns the only keeper. `open` returns `ShmHandle`s. +- While the backing file exists, a process that knows the path can open the shared memory. +- `remove` deletes the backing file's name, so later opens fail. This is cleanup, not a stop signal: processes that already opened the shared memory keep reading and writing. The fspy channel stops writers with the close gate it stores in the shared bytes. +- An `ShmHandle` and its `Mapping`s stay usable after the backing file is removed. They keep the bytes alive and cannot restore the path. -- While the keeper is alive, a process that knows the identifier can open the shared memory. -- Dropping the keeper removes the backing file's name, so later opens fail. This is cleanup, not a stop signal: processes that already opened the shared memory keep reading and writing. The fspy channel stops writers with the close gate it stores in the shared bytes. -- An `ShmHandle` and its `Mapping`s stay usable after the keeper is gone. They keep the bytes alive and cannot extend the identifier's validity. +The channel guards the same window from its own side: [`ChannelConf::sender`](../fspy_shared/src/ipc/channel/mod.rs) opens and locks the receiver's exact lock-file path before it calls `fspy_shm::open`, and the receiver removes that path before removing the backing file, so a sender that starts later fails before opening shared memory. -The channel guards the same window from its own side: [`ChannelConf::sender`](../fspy_shared/src/ipc/channel/mod.rs) opens and locks the receiver's exact lock-file path before it calls `fspy_shm::open`, and the receiver removes that path before dropping the keeper, so a sender that starts later fails before opening shared memory. - -If the keeper's process is killed, its `Drop` never runs and the file stays behind: on Unix for the system's temporary-file reaper, on Windows until a cleanup tool runs. The file costs about as much disk as the run wrote into it. +If the process that owns the path is killed before it calls `remove`, the file stays behind: on Unix for the system's temporary-file reaper, on Windows until a cleanup tool runs. The file costs about as much disk as the run wrote into it. diff --git a/crates/fspy_shm/src/lib.rs b/crates/fspy_shm/src/lib.rs index 4d232fdde..65126f27b 100644 --- a/crates/fspy_shm/src/lib.rs +++ b/crates/fspy_shm/src/lib.rs @@ -5,60 +5,79 @@ mod unix; #[cfg(windows)] mod windows; -pub use platform::{Mapping, ShmHandle, ShmKeeper, create, open}; +pub use platform::{Mapping, ShmHandle, create, open, remove}; #[cfg(unix)] use unix as platform; #[cfg(windows)] use windows as platform; -/// Prefix of backing file names inside the system temporary directory. -/// -/// The files sit directly in the temporary directory. A shared subdirectory -/// would belong to whichever user created it first and block everyone else; -/// uniquely named `0o600` files in the sticky-bit temp directory avoid that. -const BACKING_PREFIX: &str = "vite-task-fspy-"; - #[cfg(test)] mod tests { #[cfg(windows)] use std::fs::File; - use std::{ - env::temp_dir, - ffi::{OsStr, OsString}, - fs, - mem::align_of, - path::Path, - process::Command, - }; + use std::{env::temp_dir, ffi::OsStr, mem::align_of, path::PathBuf, process::Command}; use subprocess_test::command_for_fn; use uuid::Uuid; - use super::{BACKING_PREFIX, Mapping, create, open}; + use super::{Mapping, create, open, remove}; // Page-aligned on all supported targets. const SIZE: usize = 64 * 1024; // Use one byte more than 64 KiB to test multiple pages and a partial last page. const ZERO_INITIALIZED_SIZE: usize = SIZE + 1; + /// A fresh backing path that removes its file when the test ends, even on + /// panic — the job the fspy channel's keeper does in production. + struct BackingPath(PathBuf); + + impl BackingPath { + fn new() -> Self { + let path = std::path::absolute(temp_dir()) + .unwrap() + .join(format!("fspy-shm-test-{}.shm", Uuid::new_v4().simple())); + Self(path) + } + + fn as_os_str(&self) -> &OsStr { + self.0.as_os_str() + } + + fn exists(&self) -> bool { + self.0.exists() + } + + fn to_str(&self) -> String { + self.0.to_str().expect("test temp dir is UTF-8").to_owned() + } + } + + impl Drop for BackingPath { + fn drop(&mut self) { + let _ = remove(self.0.as_os_str()); + } + } + #[test] fn new_mapping_is_zero_initialized_in_all_views() { - let (keeper, handle) = create(ZERO_INITIALIZED_SIZE).unwrap(); + let path = BackingPath::new(); + let handle = create(path.as_os_str(), ZERO_INITIALIZED_SIZE).unwrap(); let first = handle.map().unwrap(); - let second = open(keeper.id()).unwrap().map().unwrap(); + let second = open(path.as_os_str()).unwrap().map().unwrap(); assert_zero_initialized(&first); assert_zero_initialized(&second); } #[test] - fn mappings_of_one_keeper_are_shared() { - let (keeper, handle) = create(SIZE).unwrap(); + fn mappings_of_one_backing_file_are_shared() { + let path = BackingPath::new(); + let handle = create(path.as_os_str(), SIZE).unwrap(); let first = handle.map().unwrap(); assert_eq!(first.len(), SIZE); assert_eq!(first.as_ptr() as usize % align_of::(), 0); - let second = open(keeper.id()).unwrap().map().unwrap(); + let second = open(path.as_os_str()).unwrap().map().unwrap(); assert_eq!(second.len(), SIZE); write_byte(&first, 0, 17); @@ -69,7 +88,8 @@ mod tests { #[test] fn one_handle_maps_repeatedly() { - let (_keeper, handle) = create(SIZE).unwrap(); + let path = BackingPath::new(); + let handle = create(path.as_os_str(), SIZE).unwrap(); let first = handle.map().unwrap(); let second = handle.map().unwrap(); @@ -78,89 +98,70 @@ mod tests { } #[test] - fn mapping_is_visible_across_processes() { - let (keeper, handle) = create(SIZE).unwrap(); - let mapping = handle.map().unwrap(); - write_byte(&mapping, 0, 17); + fn create_rejects_an_existing_path() { + let path = BackingPath::new(); + let _handle = create(path.as_os_str(), SIZE).unwrap(); - let id = keeper.id().to_str().expect("test temp dir is UTF-8").to_owned(); - let command = command_for_fn!(id, |id: String| { - let opened = open(OsStr::new(&id)).unwrap().map().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(&mapping, SIZE - 1), 29); + assert!(create(path.as_os_str(), SIZE).is_err()); } #[test] - fn subprocess_open_ignores_changed_temp_and_working_directory() { - let (keeper, handle) = create(SIZE).unwrap(); + fn mapping_is_visible_across_processes() { + let path = BackingPath::new(); + let handle = create(path.as_os_str(), SIZE).unwrap(); let mapping = handle.map().unwrap(); - let changed_cwd = - temp_dir().join(format!("{BACKING_PREFIX}changed-cwd-{}", Uuid::new_v4())); - fs::create_dir(&changed_cwd).unwrap(); write_byte(&mapping, 0, 17); - let id = keeper.id().to_str().expect("test temp dir is UTF-8").to_owned(); - let mut command = command_for_fn!(id, |id: String| { - let opened = open(OsStr::new(&id)).unwrap().map().unwrap(); + let command = command_for_fn!(path.to_str(), |path: String| { + let opened = open(OsStr::new(&path)).unwrap().map().unwrap(); assert_eq!(read_byte(&opened, 0), 17); write_byte(&opened, SIZE - 1, 29); }); - command.cwd = changed_cwd.clone(); - // The identifier is an absolute path, so a relative temporary directory - // in the child must make no difference on any platform. - for name in ["TMPDIR", "TMP", "TEMP"] { - command.envs.insert(OsString::from(name), OsString::from("changed-relative-tmp")); - } - let succeeded = Command::from(command).status().unwrap().success(); - fs::remove_dir(changed_cwd).unwrap(); - - assert!(succeeded); + assert!(Command::from(command).status().unwrap().success()); assert_eq!(read_byte(&mapping, SIZE - 1), 29); } #[test] - fn keeper_drop_prevents_new_opens() { - let (keeper, handle) = create(SIZE).unwrap(); - let id = keeper.id().to_owned(); + fn remove_prevents_new_opens() { + let path = BackingPath::new(); + let handle = create(path.as_os_str(), SIZE).unwrap(); drop(handle); - drop(keeper); - assert!(open(&id).is_err()); + remove(path.as_os_str()).unwrap(); + + assert!(open(path.as_os_str()).is_err()); } #[test] - fn opened_mapping_survives_keeper_drop() { - let (keeper, handle) = create(SIZE).unwrap(); - let id = keeper.id().to_owned(); - let opened = open(&id).unwrap().map().unwrap(); + fn opened_mapping_survives_remove() { + let path = BackingPath::new(); + let handle = create(path.as_os_str(), SIZE).unwrap(); + let opened = open(path.as_os_str()).unwrap().map().unwrap(); write_byte(&opened, 0, 17); drop(handle); - drop(keeper); - assert!(open(&id).is_err()); + remove(path.as_os_str()).unwrap(); + + assert!(open(path.as_os_str()).is_err()); assert_eq!(read_byte(&opened, 0), 17); write_byte(&opened, SIZE - 1, 29); assert_eq!(read_byte(&opened, SIZE - 1), 29); } /// Removal semantics, part one: a mapping alone (no handle) keeps the - /// bytes alive across the keeper's removal of the name. + /// bytes alive across the removal of the name. #[test] - fn keeper_drop_removes_backing_file_and_preserves_existing_mappings() { - let (keeper, handle) = create(SIZE).unwrap(); - let id = keeper.id().to_owned(); - let path = Path::new(&id).to_owned(); - let opened = open(&id).unwrap().map().unwrap(); + fn remove_deletes_backing_file_and_preserves_existing_mappings() { + let path = BackingPath::new(); + let handle = create(path.as_os_str(), SIZE).unwrap(); + let opened = open(path.as_os_str()).unwrap().map().unwrap(); drop(handle); assert!(path.exists()); - drop(keeper); + remove(path.as_os_str()).unwrap(); assert!(!path.exists()); - assert!(open(&id).is_err()); + assert!(open(path.as_os_str()).is_err()); write_byte(&opened, 0, 17); assert_eq!(read_byte(&opened, 0), 17); } @@ -168,16 +169,16 @@ mod tests { /// Removal semantics, part two: the name goes away even while a handle is /// still open, and that handle keeps mapping the same bytes afterwards. #[test] - fn keeper_drop_with_open_handle_removes_name_and_handle_still_maps() { - let (keeper, handle) = create(SIZE).unwrap(); - let id = keeper.id().to_owned(); + fn remove_with_open_handle_removes_name_and_handle_still_maps() { + let path = BackingPath::new(); + let handle = create(path.as_os_str(), SIZE).unwrap(); let before = handle.map().unwrap(); write_byte(&before, 0, 17); - drop(keeper); + remove(path.as_os_str()).unwrap(); - assert!(!Path::new(&id).exists()); - assert!(open(&id).is_err()); + assert!(!path.exists()); + assert!(open(path.as_os_str()).is_err()); let after = handle.map().unwrap(); assert_eq!(read_byte(&after, 0), 17); @@ -192,16 +193,17 @@ mod tests { #[cfg(windows)] const MAX_ENDPOINT_ALLOCATION: u64 = 16 * 1024 * 1024; - let (keeper, handle) = create(PRODUCTION_SIZE).unwrap(); + let path = BackingPath::new(); + let handle = create(path.as_os_str(), PRODUCTION_SIZE).unwrap(); #[cfg(windows)] { - let (logical_size, initial_allocation) = backing_file_sizes(keeper.id()); + let (logical_size, initial_allocation) = backing_file_sizes(path.as_os_str()); assert_eq!(logical_size, PRODUCTION_SIZE as u64); assert!(initial_allocation < MAX_ENDPOINT_ALLOCATION); } let first = handle.map().unwrap(); - let opened = open(keeper.id()).unwrap().map().unwrap(); + let opened = open(path.as_os_str()).unwrap().map().unwrap(); write_byte(&first, 0, 17); write_byte(&first, PRODUCTION_SIZE - 1, 29); assert_eq!(read_byte(&opened, 0), 17); @@ -210,15 +212,15 @@ mod tests { // Touching both endpoints must not have allocated the range between them. #[cfg(windows)] { - let (logical_size, endpoint_allocation) = backing_file_sizes(keeper.id()); + let (logical_size, endpoint_allocation) = backing_file_sizes(path.as_os_str()); assert_eq!(logical_size, PRODUCTION_SIZE as u64); assert!(endpoint_allocation < MAX_ENDPOINT_ALLOCATION); } } #[cfg(windows)] - fn backing_file_sizes(id: &OsStr) -> (u64, u64) { - let file = File::open(id).unwrap(); + fn backing_file_sizes(path: &OsStr) -> (u64, u64) { + let file = File::open(path).unwrap(); super::windows::file_sizes(&file).unwrap() } diff --git a/crates/fspy_shm/src/unix.rs b/crates/fspy_shm/src/unix.rs index 38c56cbb8..22200abac 100644 --- a/crates/fspy_shm/src/unix.rs +++ b/crates/fspy_shm/src/unix.rs @@ -2,28 +2,13 @@ //! path. use std::{ - env::temp_dir, ffi::{CString, OsStr}, io, num::NonZeroUsize, os::unix::ffi::OsStrExt as _, - path::PathBuf, ptr::{self, NonNull}, }; -use uuid::Uuid; - -use crate::BACKING_PREFIX; - -/// Keeps the shared memory's identifier alive and removes it on drop. -/// -/// Removal is cleanup, not a stop signal: later opens fail, but existing -/// [`ShmHandle`]s and [`Mapping`]s keep reading and writing. To stop them, -/// store a flag in the shared bytes, as the fspy channel's close gate does. -pub struct ShmKeeper { - path: PathBuf, -} - /// Opened shared memory that is not mapped yet. /// /// [`map`](Self::map) can be called more than once; every call returns another @@ -49,18 +34,24 @@ unsafe impl Send for Mapping {} // concurrent access is synchronized by the fspy channel. unsafe impl Sync for Mapping {} -/// Creates `size` bytes of zero-initialized shared memory. +/// Creates `size` bytes of zero-initialized shared memory backed by the file +/// at `path`. +/// +/// The file must not exist yet and is created with mode `0o600`. The path is +/// how other processes reach the shared memory, so the caller should supply +/// an absolute path that keeps working across working-directory changes. /// -/// Returns its [`ShmKeeper`] and an already opened [`ShmHandle`], so the -/// creating process never has to go through [`open`]. +/// Returns an already opened [`ShmHandle`], so the creating process never has +/// to look its own file up by path. /// /// Only pages that are actually written occupy memory or disk, so a large /// capacity is cheap. /// /// # Errors /// -/// Returns an error if the shared memory cannot be created or sized. -pub fn create(size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { +/// Returns an error if the shared memory cannot be created or sized. A file +/// created by the failing call is removed before it returns. +pub fn create(path: &OsStr, size: usize) -> io::Result { let size = NonZeroUsize::new(size).ok_or_else(|| { io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size must be nonzero") })?; @@ -68,14 +59,8 @@ pub fn create(size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size exceeds u64") })?; - // `temp_dir` reflects `TMPDIR` verbatim, which may be relative. The - // identifier travels to processes with other working directories, so - // resolve it against the creator's current directory first. - let path = std::path::absolute(temp_dir())? - .join(format!("{BACKING_PREFIX}{}.shm", Uuid::new_v4().simple())); - let file = open_file( - path.as_os_str(), + path, fspy_nostd::fs::OFlags::RDWR | fspy_nostd::fs::OFlags::CREATE | fspy_nostd::fs::OFlags::EXCL @@ -83,27 +68,26 @@ pub fn create(size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { // Only the creating user may open the mapping. fspy_nostd::fs::Mode::RUSR | fspy_nostd::fs::Mode::WUSR, )?; - // The keeper exists from here on, so every error path below cleans up. - let keeper = ShmKeeper { path }; // Every byte reads as zero because the file is all holes. - fspy_nostd::fs::ftruncate(&file, size_u64).map_err(error_to_io)?; + if let Err(error) = fspy_nostd::fs::ftruncate(&file, size_u64) { + // Do not hand the caller an unusable partial file to clean up. + let _ = remove(path); + return Err(error_to_io(error)); + } - Ok((keeper, ShmHandle { file, size })) + Ok(ShmHandle { file, size }) } -/// Opens the shared memory identified by `id`. -/// -/// The identifier works from any process, regardless of the process's working -/// directory or environment. +/// Opens the shared memory backed by the file at `path`. /// /// # Errors /// /// Returns an error if the shared memory is unavailable, which is the common -/// case once its keeper has been dropped. -pub fn open(id: &OsStr) -> io::Result { +/// case once the backing file has been removed. +pub fn open(path: &OsStr) -> io::Result { let file = open_file( - id, + path, fspy_nostd::fs::OFlags::RDWR | fspy_nostd::fs::OFlags::CLOEXEC, fspy_nostd::fs::Mode::empty(), )?; @@ -126,7 +110,16 @@ fn open_file( fspy_nostd::fs::openat(fspy_nostd::CWD, as_nostd_path(&path), flags, mode).map_err(error_to_io) } -fn remove_file(path: &OsStr) -> io::Result<()> { +/// Removes the shared memory at `path`. +/// +/// Removal is cleanup, not a stop signal: later opens fail, but existing +/// [`ShmHandle`]s and [`Mapping`]s keep reading and writing. To stop them, +/// store a flag in the shared bytes, as the fspy channel's close gate does. +/// +/// # Errors +/// +/// Returns the error reported while unlinking the path. +pub fn remove(path: &OsStr) -> io::Result<()> { let path = CString::new(path.as_bytes())?; fspy_nostd::fs::unlinkat( fspy_nostd::CWD, @@ -146,21 +139,6 @@ fn error_to_io(error: fspy_nostd::Error) -> io::Error { io::Error::from_raw_os_error(error.raw_os_error()) } -impl Drop for ShmKeeper { - fn drop(&mut self) { - let _ = remove_file(self.path.as_os_str()); - } -} - -impl ShmKeeper { - /// Returns the shared memory's opaque identifier, which any process passes - /// to [`open`]. - #[must_use] - pub fn id(&self) -> &OsStr { - self.path.as_os_str() - } -} - impl ShmHandle { /// Maps the shared bytes. /// diff --git a/crates/fspy_shm/src/windows.rs b/crates/fspy_shm/src/windows.rs index 8a7582a8d..0fcb19da0 100644 --- a/crates/fspy_shm/src/windows.rs +++ b/crates/fspy_shm/src/windows.rs @@ -2,14 +2,7 @@ //! its path. use core::{ffi::c_void, mem::size_of, ptr}; -use std::{ - env::temp_dir, - ffi::OsStr, - io, - num::NonZeroUsize, - os::windows::ffi::OsStrExt as _, - path::{Path, PathBuf}, -}; +use std::{ffi::OsStr, io, num::NonZeroUsize, os::windows::ffi::OsStrExt as _}; #[cfg(test)] use std::{fs::File, os::windows::io::AsRawHandle as _}; @@ -18,8 +11,6 @@ use fspy_nostd::{ fs::{CreationDisposition, FileAccess, FileOptions, FileShare}, mm::{MappingAccess, PageProtection}, }; -use omnipath::windows::WinPathExt as _; -use uuid::Uuid; #[cfg(test)] use windows_sys::Win32::Storage::FileSystem::{ FILE_STANDARD_INFO, FileStandardInfo, GetFileInformationByHandleEx, @@ -33,23 +24,8 @@ use windows_sys::Win32::{ System::{IO::DeviceIoControl, Ioctl::FSCTL_SET_SPARSE}, }; -use crate::BACKING_PREFIX; - const SHARE_ALL: FileShare = FileShare::READ.union(FileShare::WRITE).union(FileShare::DELETE); -/// Keeps the shared memory's identifier alive and removes it on drop. -/// -/// Removal relies on POSIX delete semantics, which requires NTFS on Windows -/// 10 1607 or newer: the name is unlinked immediately even while views of the -/// backing file remain mapped. -/// -/// Removal is cleanup, not a stop signal: later opens fail, but existing -/// [`ShmHandle`]s and [`Mapping`]s keep reading and writing. To stop them, -/// store a flag in the shared bytes, as the fspy channel's close gate does. -pub struct ShmKeeper { - path: PathBuf, -} - /// Opened shared memory that is not mapped yet. /// /// [`map`](Self::map) can be called more than once; every call returns another @@ -68,10 +44,18 @@ pub struct Mapping { len: NonZeroUsize, } -/// Creates `size` bytes of zero-initialized shared memory. +/// Creates `size` bytes of zero-initialized shared memory backed by the file +/// at `path`. +/// +/// The file must not exist yet; the per-user `%TEMP%` ACL provides same-user +/// gating when `path` sits in the temporary directory. The path is how other +/// processes reach the shared memory, so the caller should supply an absolute +/// path that keeps working across working-directory changes. Paths at or +/// beyond the legacy `MAX_PATH` limit must already be in verbatim (`\\?\`) +/// form: this crate passes paths to the OS unchanged. /// -/// Returns its [`ShmKeeper`] and an already opened [`ShmHandle`], so the -/// creating process never has to go through [`open`]. +/// Returns an already opened [`ShmHandle`], so the creating process never has +/// to look its own file up by path. /// /// Only pages that are actually written occupy memory or disk, so a large /// capacity is cheap. @@ -79,8 +63,9 @@ pub struct Mapping { /// # Errors /// /// Returns an error if the shared memory cannot be created or sized. Creation -/// fails on volumes without sparse-file support. -pub fn create(size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { +/// fails on volumes without sparse-file support. A file created by the +/// failing call is removed before it returns. +pub fn create(path: &OsStr, size: usize) -> io::Result { let size = NonZeroUsize::new(size).ok_or_else(|| { io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size must be nonzero") })?; @@ -88,12 +73,8 @@ pub fn create(size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size exceeds i64") })?; - // The per-user `%TEMP%` ACL provides same-user gating. The identifier is - // absolute so it keeps working after a working-directory change. - let path = std::path::absolute(temp_dir())? - .join(format!("{BACKING_PREFIX}{}.shm", Uuid::new_v4().simple())); let file = open_file( - path.as_os_str(), + path, FileAccess::GENERIC_READ | FileAccess::GENERIC_WRITE, CreationDisposition::CreateNew, // Ask Windows to keep the data in memory when it can. Opening the @@ -101,31 +82,34 @@ pub fn create(size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { // instead of redirecting the file, as std does for `create_new`. FileOptions::TEMPORARY | FileOptions::OPEN_REPARSE_POINT, )?; - // The keeper exists from here on, so every error path below cleans up. - let keeper = ShmKeeper { path }; + if let Err(error) = size_backing_file(file.as_handle(), size_i64) { + // Do not hand the caller an unusable partial file to clean up. + let _ = remove(path); + return Err(error_to_io(error)); + } + + Ok(ShmHandle { file, size }) +} + +fn size_backing_file(file: BorrowedHandle<'_>, len: i64) -> fspy_nostd::Result<()> { // NTFS allocates clusters for the whole logical size unless the file is // marked sparse first, which would turn the capacity into real disk usage. // Volumes without sparse-file support fail here. - set_sparse(file.as_handle()).map_err(error_to_io)?; + set_sparse(file)?; // Every byte reads as zero because the file is all holes. - set_end_of_file(file.as_handle(), size_i64).map_err(error_to_io)?; - - Ok((keeper, ShmHandle { file, size })) + set_end_of_file(file, len) } -/// Opens the shared memory identified by `id`. -/// -/// The identifier works from any process, regardless of the process's working -/// directory or environment. +/// Opens the shared memory backed by the file at `path`. /// /// # Errors /// /// Returns an error if the shared memory is unavailable, which is the common -/// case once its keeper has been dropped. -pub fn open(id: &OsStr) -> io::Result { +/// case once the backing file has been removed. +pub fn open(path: &OsStr) -> io::Result { let file = open_file( - id, + path, FileAccess::GENERIC_READ | FileAccess::GENERIC_WRITE, CreationDisposition::OpenExisting, FileOptions::empty(), @@ -162,22 +146,11 @@ fn open_file_wide( fspy_nostd::fs::create_file(path, access, SHARE_ALL, None, disposition, options, None) } -/// The length at which std's own Windows path conversion switches to a -/// verbatim path. -const VERBATIM_THRESHOLD: usize = 248; - fn copy_path(path: &OsStr) -> io::Result> { let mut units: Vec<_> = path.encode_wide().collect(); if units.contains(&0) { return Err(io::Error::new(io::ErrorKind::InvalidInput, "path contains NUL")); } - // std converts long paths to verbatim form before every `CreateFileW`; - // without that, paths at or beyond the legacy `MAX_PATH` limit fail - // regardless of the system's long-path opt-in, which the arbitrary - // processes opening shared memory could not rely on anyway. - if units.len() >= VERBATIM_THRESHOLD { - units = Path::new(path).to_verbatim()?.as_os_str().encode_wide().collect(); - } units.push(0); Ok(units) } @@ -229,7 +202,20 @@ fn set_end_of_file(file: BorrowedHandle<'_>, len: i64) -> fspy_nostd::Result<()> }) } -fn remove_file(path: &OsStr) -> io::Result<()> { +/// Removes the shared memory at `path`. +/// +/// Removal relies on POSIX delete semantics, which requires NTFS on Windows +/// 10 1607 or newer: the name is unlinked immediately even while views of the +/// backing file remain mapped. +/// +/// Removal is cleanup, not a stop signal: later opens fail, but existing +/// [`ShmHandle`]s and [`Mapping`]s keep reading and writing. To stop them, +/// store a flag in the shared bytes, as the fspy channel's close gate does. +/// +/// # Errors +/// +/// Returns the error reported while unlinking the path. +pub fn remove(path: &OsStr) -> io::Result<()> { let path = copy_path(path)?; // Opening the reparse point itself removes a link rather than its target. let file = open_file_wide( @@ -266,21 +252,6 @@ fn set_posix_delete(file: BorrowedHandle<'_>) -> fspy_nostd::Result<()> { }) } -impl Drop for ShmKeeper { - fn drop(&mut self) { - let _ = remove_file(self.path.as_os_str()); - } -} - -impl ShmKeeper { - /// Returns the shared memory's opaque identifier, which any process passes - /// to [`open`]. - #[must_use] - pub fn id(&self) -> &OsStr { - self.path.as_os_str() - } -} - impl ShmHandle { /// Maps the shared bytes. ///