diff --git a/crates/fspy_shared/src/ipc/channel/mod.rs b/crates/fspy_shared/src/ipc/channel/mod.rs index fae0f045d..9a07f77b9 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; @@ -28,15 +28,21 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> { let lock_file_path = temp_dir.join(format!("fspy_ipc_{id}.lock")); let shm_path = temp_dir.join(format!("fspy_ipc_{id}.shm")); - let (keeper, handle) = fspy_shm::create(shm_path.as_os_str(), capacity)?; - let mapping = handle.map()?; + let handle = fspy_shm::create(shm_path.as_os_str(), capacity)?; + let mapping = match handle.map() { + Ok(mapping) => mapping, + Err(error) => { + let _ = fspy_shm::remove(shm_path.as_os_str()); + return Err(error); + } + }; let conf = ChannelConf { lock_file_path: lock_file_path.as_os_str().into(), shm_path: shm_path.as_os_str().into(), }; - let receiver = Receiver::new(lock_file_path, keeper, mapping)?; + let receiver = Receiver::new(lock_file_path, shm_path, mapping)?; Ok((conf, receiver)) } @@ -94,9 +100,8 @@ unsafe impl Sync for Sender {} /// Owns the lock file and removes it on drop. pub struct Receiver { lock_file_path: PathBuf, + shm_path: PathBuf, lock_file: File, - /// Keeps the backing file's name alive for as long as senders may attach. - _keeper: ShmKeeper, mapping: Mapping, } @@ -111,13 +116,22 @@ impl Drop for Receiver { if let Err(err) = std::fs::remove_file(&self.lock_file_path) { debug!("Failed to remove IPC lock file {}: {}", self.lock_file_path.display(), err); } + if let Err(err) = fspy_shm::remove(self.shm_path.as_os_str()) { + debug!("Failed to remove IPC shared memory {}: {}", self.shm_path.display(), err); + } } } impl Receiver { - fn new(lock_file_path: PathBuf, keeper: ShmKeeper, mapping: Mapping) -> io::Result { - let lock_file = File::create(&lock_file_path)?; - Ok(Self { lock_file_path, lock_file, _keeper: keeper, mapping }) + fn new(lock_file_path: PathBuf, shm_path: PathBuf, mapping: Mapping) -> io::Result { + let lock_file = match File::create(&lock_file_path) { + Ok(lock_file) => lock_file, + Err(error) => { + let _ = fspy_shm::remove(shm_path.as_os_str()); + return Err(error); + } + }; + Ok(Self { lock_file_path, shm_path, lock_file, mapping }) } /// Lock the shared memory for unique read access. diff --git a/crates/fspy_shared/src/ipc/channel/shm_io.rs b/crates/fspy_shared/src/ipc/channel/shm_io.rs index 0dd585ca7..4db0f6390 100644 --- a/crates/fspy_shared/src/ipc/channel/shm_io.rs +++ b/crates/fspy_shared/src/ipc/channel/shm_io.rs @@ -675,7 +675,7 @@ mod tests { let shm_path = std::path::absolute(std::env::temp_dir()) .unwrap() .join(format!("fspy_ipc_test_{}.shm", uuid::Uuid::new_v4())); - let (_keeper, handle) = fspy_shm::create(shm_path.as_os_str(), SHM_SIZE).unwrap(); + let handle = fspy_shm::create(shm_path.as_os_str(), SHM_SIZE).unwrap(); 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 @@ -720,5 +720,6 @@ mod tests { assert!(frames.contains(&BStr::new(frame_data.as_bytes()))); } } + fspy_shm::remove(shm_path.as_os_str()).unwrap(); } } diff --git a/crates/fspy_shm/README.md b/crates/fspy_shm/README.md index cdcb557e6..ebee96faf 100644 --- a/crates/fspy_shm/README.md +++ b/crates/fspy_shm/README.md @@ -10,14 +10,15 @@ The public API is defined in [`src/lib.rs`](src/lib.rs). | API | Contract | | --------------------- | --------------------------------------------------------------------------------------- | -| `create(path, size)` | Creates a zero-initialized backing file and returns its `ShmKeeper` and an `ShmHandle`. | +| `create(path, size)` | Creates a zero-initialized backing file and returns an `ShmHandle`. | | `open(path)` | Opens an `ShmHandle` on the shared memory at the same path. | +| `remove(path)` | Removes the name while preserving existing handles and mappings. | | `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` owns the name's lifetime: 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. +The caller owns the backing-file name and decides when to remove it. `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. Neither type 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. @@ -27,18 +28,18 @@ One implementation serves every platform: a sparse file at the full path supplie 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. -Unix opens and maps through `sigsafe`; on Linux, its raw-syscall backend works before libc initialization. Windows maps through `memmap2`. The remaining platform-specific parts are short passages: +Unix creates, sizes, opens, maps, and unlinks through `sigsafe`; on Linux, its raw-syscall backend works before libc initialization. Windows maps through `memmap2`. The remaining platform-specific parts are 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 | +| Explicit removal | unlink the path | unlink the path; see the fallback below | | Descriptor safety | raw `openat` with `O_CLOEXEC` | non-inheritable handles, the Rust standard default | `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. +Unix `remove` unlinks through `sigsafe`. Windows first uses `remove_file`. 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. `remove` 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. ## Options considered @@ -60,12 +61,12 @@ Earlier revisions rejected temporary files because dirty pages can reach disk. O ## Lifetime semantics -`create(path, size)` returns the only keeper. `open(path)` returns `ShmHandle`s. +`create(path, size)` and `open(path)` return `ShmHandle`s. The caller retains the path and passes it to `remove` at the end of the attachment window. -- While the keeper is alive, a process that knows the path 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 path's validity. +- Until `remove` succeeds, a process that knows the path can open the shared memory. +- Removal makes 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 removal. They keep the bytes alive and cannot restore the path. -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. +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 the lock path before removing the shared-memory path, 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. The fspy channel places it in the system temporary directory, where Unix temporary-file reapers or Windows cleanup tools can remove it. The file costs about as much disk as the run wrote into it. +If the receiver's process is killed, cleanup never runs and the file stays behind. The fspy channel places it in the system temporary directory, where Unix temporary-file reapers or Windows cleanup tools can remove it. 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 312574d30..e6ff38f0a 100644 --- a/crates/fspy_shm/src/lib.rs +++ b/crates/fspy_shm/src/lib.rs @@ -5,7 +5,7 @@ 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)] @@ -27,7 +27,7 @@ mod tests { use subprocess_test::command_for_fn; use uuid::Uuid; - use super::{Mapping, ShmHandle, ShmKeeper, create as create_at, open}; + use super::{Mapping, ShmHandle, create as create_at, open, remove}; const TEST_BACKING_PREFIX: &str = "vite-task-fspy-test-"; // Page-aligned on all supported targets. @@ -37,7 +37,7 @@ mod tests { #[test] fn new_mapping_is_zero_initialized_in_all_views() { - let (id, _keeper, handle) = create(ZERO_INITIALIZED_SIZE); + let (id, _cleanup, handle) = create(ZERO_INITIALIZED_SIZE); let first = handle.map().unwrap(); let second = open(&id).unwrap().map().unwrap(); @@ -46,8 +46,8 @@ mod tests { } #[test] - fn mappings_of_one_keeper_are_shared() { - let (id, _keeper, handle) = create(SIZE); + fn mappings_of_one_backing_file_are_shared() { + let (id, _cleanup, handle) = create(SIZE); let first = handle.map().unwrap(); assert_eq!(first.len(), SIZE); assert_eq!(first.as_ptr() as usize % align_of::(), 0); @@ -63,7 +63,7 @@ mod tests { #[test] fn one_handle_maps_repeatedly() { - let (_id, _keeper, handle) = create(SIZE); + let (_id, _cleanup, handle) = create(SIZE); let first = handle.map().unwrap(); let second = handle.map().unwrap(); @@ -73,7 +73,7 @@ mod tests { #[test] fn mapping_is_visible_across_processes() { - let (id, _keeper, handle) = create(SIZE); + let (id, _cleanup, handle) = create(SIZE); let mapping = handle.map().unwrap(); write_byte(&mapping, 0, 17); @@ -89,7 +89,7 @@ mod tests { #[test] fn subprocess_open_ignores_changed_temp_and_working_directory() { - let (id, _keeper, handle) = create(SIZE); + let (id, _cleanup, handle) = create(SIZE); let mapping = handle.map().unwrap(); let changed_cwd = temp_dir().join(format!("{TEST_BACKING_PREFIX}changed-cwd-{}", Uuid::new_v4())); @@ -116,21 +116,21 @@ mod tests { } #[test] - fn keeper_drop_prevents_new_opens() { - let (id, keeper, handle) = create(SIZE); + fn remove_prevents_new_opens() { + let (id, _cleanup, handle) = create(SIZE); drop(handle); - drop(keeper); + remove(&id).unwrap(); assert!(open(&id).is_err()); } #[test] - fn opened_mapping_survives_keeper_drop() { - let (id, keeper, handle) = create(SIZE); + fn opened_mapping_survives_remove() { + let (id, _cleanup, handle) = create(SIZE); let opened = open(&id).unwrap().map().unwrap(); write_byte(&opened, 0, 17); drop(handle); - drop(keeper); + remove(&id).unwrap(); assert!(open(&id).is_err()); assert_eq!(read_byte(&opened, 0), 17); @@ -139,16 +139,16 @@ mod tests { } /// Removal semantics, part one: a mapping alone (no handle) keeps the - /// bytes alive across the keeper's removal of the name. + /// bytes alive after removal of the name. #[test] - fn keeper_drop_removes_backing_file_and_preserves_existing_mappings() { - let (id, keeper, handle) = create(SIZE); + fn remove_backing_file_preserves_existing_mappings() { + let (id, _cleanup, handle) = create(SIZE); let path = Path::new(&id).to_owned(); let opened = open(&id).unwrap().map().unwrap(); drop(handle); assert!(path.exists()); - drop(keeper); + remove(&id).unwrap(); assert!(!path.exists()); assert!(open(&id).is_err()); @@ -159,12 +159,12 @@ 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 (id, keeper, handle) = create(SIZE); + fn remove_with_open_handle_preserves_handle() { + let (id, _cleanup, handle) = create(SIZE); let before = handle.map().unwrap(); write_byte(&before, 0, 17); - drop(keeper); + remove(&id).unwrap(); assert!(!Path::new(&id).exists()); assert!(open(&id).is_err()); @@ -182,7 +182,7 @@ mod tests { #[cfg(windows)] const MAX_ENDPOINT_ALLOCATION: u64 = 16 * 1024 * 1024; - let (id, _keeper, handle) = create(PRODUCTION_SIZE); + let (id, _cleanup, handle) = create(PRODUCTION_SIZE); #[cfg(windows)] { let (logical_size, initial_allocation) = backing_file_sizes(&id); @@ -212,13 +212,22 @@ mod tests { super::windows::file_sizes(&file).unwrap() } - fn create(size: usize) -> (OsString, ShmKeeper, ShmHandle) { + struct Cleanup(OsString); + + impl Drop for Cleanup { + fn drop(&mut self) { + let _ = remove(&self.0); + } + } + + fn create(size: usize) -> (OsString, Cleanup, ShmHandle) { let path = std::path::absolute(temp_dir()) .unwrap() .join(format!("{TEST_BACKING_PREFIX}{}.shm", Uuid::new_v4().simple())); let id = path.into_os_string(); - let (keeper, handle) = create_at(&id, size).unwrap(); - (id, keeper, handle) + let handle = create_at(&id, size).unwrap(); + let cleanup = Cleanup(id.clone()); + (id, cleanup, handle) } fn read_byte(mapping: &Mapping, index: usize) -> u8 { diff --git a/crates/fspy_shm/src/unix.rs b/crates/fspy_shm/src/unix.rs index 584b3c20a..476b0caff 100644 --- a/crates/fspy_shm/src/unix.rs +++ b/crates/fspy_shm/src/unix.rs @@ -2,22 +2,12 @@ use std::{ ffi::OsStr, - fs, io, + io, num::NonZeroUsize, os::unix::ffi::OsStrExt as _, - path::PathBuf, ptr::{self, NonNull}, }; -/// Keeps the shared memory's backing-file name 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 @@ -44,8 +34,9 @@ unsafe impl Send for Mapping {} unsafe impl Sync for Mapping {} /// Creates `size` bytes of zero-initialized shared memory at `path`. /// -/// 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 go through [`open`]. The caller owns the backing path and must eventually +/// pass it to [`remove`]. /// /// Only pages that are actually written occupy memory or disk, so a large /// capacity is cheap. @@ -53,38 +44,37 @@ unsafe impl Sync for Mapping {} /// # Errors /// /// Returns an error if the shared memory cannot be created or sized. -pub fn create(path: &OsStr, size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { +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") })?; let size_u64 = u64::try_from(size.get()).map_err(|_| { io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size exceeds u64") })?; - let path = PathBuf::from(path); - let file = open_file( - path.as_os_str(), + path, sigsafe::fs::OFlags::RDWR | sigsafe::fs::OFlags::CREATE | sigsafe::fs::OFlags::EXCL | sigsafe::fs::OFlags::CLOEXEC, sigsafe::fs::Mode::RUSR | sigsafe::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. - sigsafe::fs::ftruncate(&file, size_u64).map_err(errno_to_io)?; + if let Err(error) = sigsafe::fs::ftruncate(&file, size_u64) { + drop(file); + let _ = remove(path); + return Err(errno_to_io(error)); + } - Ok((keeper, ShmHandle { file, size })) + Ok(ShmHandle { file, size }) } /// Opens the shared memory at `path`. /// /// # Errors /// -/// Returns an error if the shared memory is unavailable, which is the common -/// case once its keeper has been dropped. +/// Returns an error if the shared memory is unavailable, including after its +/// backing-file name has been removed. pub fn open(path: &OsStr) -> io::Result { let file = open_file( path, @@ -100,32 +90,47 @@ pub fn open(path: &OsStr) -> io::Result { Ok(ShmHandle { file, size }) } +/// Removes the shared memory's backing-file name. +/// +/// Existing handles and mappings remain usable, but later calls to [`open`] +/// fail once removal succeeds. +/// +/// # Errors +/// +/// Returns an error if the backing-file name cannot be removed. +pub fn remove(path: &OsStr) -> io::Result<()> { + let mut path_buf = [0_u8; sigsafe::fs::PATH_MAX]; + let path = copy_path(path, &mut path_buf)?; + sigsafe::fs::unlinkat(sigsafe::CWD, path, sigsafe::fs::AtFlags::empty()).map_err(errno_to_io) +} + fn open_file( path: &OsStr, flags: sigsafe::fs::OFlags, mode: sigsafe::fs::Mode, ) -> io::Result { + let mut path_buf = [0_u8; sigsafe::fs::PATH_MAX]; + let path = copy_path(path, &mut path_buf)?; + sigsafe::fs::openat(sigsafe::CWD, path, flags, mode).map_err(errno_to_io) +} + +fn copy_path<'buf>( + path: &OsStr, + buf: &'buf mut [u8; sigsafe::fs::PATH_MAX], +) -> io::Result> { let path_bytes = path.as_bytes(); let len_with_nul = path_bytes.len().checked_add(1).ok_or(io::ErrorKind::InvalidInput)?; - let mut path_buf = [0_u8; sigsafe::fs::PATH_MAX]; - let path = path_buf + let path = buf .get_mut(..len_with_nul) .ok_or_else(|| io::Error::from_raw_os_error(sigsafe::Errno::NAMETOOLONG.raw_os_error()))?; path[..path_bytes.len()].copy_from_slice(path_bytes); - let path = sigsafe::CStr::from_bytes_with_nul(path).map_err(|_| io::ErrorKind::InvalidInput)?; - sigsafe::fs::openat(sigsafe::CWD, path, flags, mode).map_err(errno_to_io) + sigsafe::CStr::from_bytes_with_nul(path).map_err(|_| io::ErrorKind::InvalidInput.into()) } fn errno_to_io(errno: sigsafe::Errno) -> io::Error { io::Error::from_raw_os_error(errno.raw_os_error()) } -impl Drop for ShmKeeper { - fn drop(&mut self) { - let _ = fs::remove_file(&self.path); - } -} - impl ShmHandle { /// Maps the shared bytes. /// diff --git a/crates/fspy_shm/src/windows.rs b/crates/fspy_shm/src/windows.rs index 705a62456..4e3242c19 100644 --- a/crates/fspy_shm/src/windows.rs +++ b/crates/fspy_shm/src/windows.rs @@ -5,7 +5,6 @@ use std::{ fs::{self, File, OpenOptions}, io, os::windows::{fs::OpenOptionsExt as _, io::AsRawHandle as _}, - path::PathBuf, }; use memmap2::{MmapOptions, MmapRaw}; @@ -26,15 +25,6 @@ const TEMPORARY: u32 = FILE_ATTRIBUTE_TEMPORARY; const DELETE_ON_CLOSE: u32 = FILE_FLAG_DELETE_ON_CLOSE; const DELETE_ACCESS: u32 = windows_sys::Win32::Storage::FileSystem::DELETE; -/// Keeps the shared memory's backing-file name 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 @@ -54,8 +44,9 @@ pub struct Mapping { /// Creates `size` bytes of zero-initialized shared memory at `path`. /// -/// 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 go through [`open`]. The caller owns the backing path and must eventually +/// pass it to [`remove`]. /// /// Only pages that are actually written occupy memory or disk, so a large /// capacity is cheap. @@ -64,7 +55,7 @@ pub struct Mapping { /// /// Returns an error if the shared memory cannot be created or sized, or the /// containing volume does not support sparse files. -pub fn create(path: &OsStr, size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { +pub fn create(path: &OsStr, size: usize) -> io::Result { if size == 0 { return Err(io::Error::new( io::ErrorKind::InvalidInput, @@ -74,8 +65,6 @@ pub fn create(path: &OsStr, size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { let size_u64 = u64::try_from(size).map_err(|_| { io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size exceeds u64") })?; - let path = PathBuf::from(path); - let file = OpenOptions::new() .read(true) .write(true) @@ -83,26 +72,30 @@ pub fn create(path: &OsStr, size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { .share_mode(SHARE_ALL) // Ask Windows to keep the data in memory when it can. .attributes(TEMPORARY) - .open(&path)?; - // The keeper exists from here on, so every error path below cleans up. - let keeper = ShmKeeper { path }; + .open(path)?; // 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)?; - // Every byte reads as zero because the file is all holes. - file.set_len(size_u64)?; + let initialized = set_sparse(&file).and_then(|()| { + // Every byte reads as zero because the file is all holes. + file.set_len(size_u64) + }); + if let Err(error) = initialized { + drop(file); + let _ = remove(path); + return Err(error); + } - Ok((keeper, ShmHandle { file, size })) + Ok(ShmHandle { file, size }) } /// Opens the shared memory at `path`. /// /// # Errors /// -/// Returns an error if the shared memory is unavailable, which is the common -/// case once its keeper has been dropped. +/// Returns an error if the shared memory is unavailable, including after its +/// backing-file name has been removed. pub fn open(path: &OsStr) -> io::Result { // Rust handles are non-inheritable, and its default share mode permits // concurrent read, write and delete access. @@ -118,20 +111,28 @@ pub fn open(path: &OsStr) -> io::Result { Ok(ShmHandle { file, size }) } -impl Drop for ShmKeeper { - fn drop(&mut self) { - // Windows versions without POSIX delete refuse to remove the name of a - // mapped file. Arm the deferred delete instead: a handle opened with - // `FILE_FLAG_DELETE_ON_CLOSE` deletes the file once every handle to it - // is closed. - if fs::remove_file(&self.path).is_err() { - let _ = OpenOptions::new() - .access_mode(DELETE_ACCESS) - .share_mode(SHARE_ALL) - .custom_flags(DELETE_ON_CLOSE) - .open(&self.path); - } +/// Removes the shared memory's backing-file name. +/// +/// Existing handles and mappings remain usable, but later calls to [`open`] +/// fail once removal succeeds. +/// +/// # Errors +/// +/// Returns an error if removal cannot be performed or scheduled. +pub fn remove(path: &OsStr) -> io::Result<()> { + if fs::remove_file(path).is_ok() { + return Ok(()); } + + // Windows versions without POSIX delete refuse to remove the name of a + // mapped file. Arm the deferred delete instead: closing this handle deletes + // the file once every other handle to it is closed. + OpenOptions::new() + .access_mode(DELETE_ACCESS) + .share_mode(SHARE_ALL) + .custom_flags(DELETE_ON_CLOSE) + .open(path) + .map(drop) } impl ShmHandle { diff --git a/crates/sigsafe/src/fs/mod.rs b/crates/sigsafe/src/fs/mod.rs index 9ee14ed9a..0b3384247 100644 --- a/crates/sigsafe/src/fs/mod.rs +++ b/crates/sigsafe/src/fs/mod.rs @@ -1,8 +1,8 @@ //! Filesystem calls with caller-owned storage. -use core::mem::MaybeUninit; +use core::{ffi::CStr as CoreCStr, mem::MaybeUninit}; -pub use rustix::fs::{Mode, OFlags, Stat, fstat, ftruncate}; +pub use rustix::fs::{AtFlags, Mode, OFlags, Stat, fstat, ftruncate}; use crate::{BorrowedFd, CStr, Fat, OwnedFd, Result}; @@ -37,6 +37,17 @@ pub fn openat( imp::openat(dirfd, path, flags, mode) } +/// Removes `path` relative to `dirfd`. +/// +/// # Errors +/// +/// Returns the error reported by `unlinkat`. +pub fn unlinkat(dirfd: BorrowedFd<'_>, path: CStr<'_, Fat>, flags: AtFlags) -> Result<()> { + // SAFETY: `path` already guarantees exactly one trailing NUL. + let path = unsafe { CoreCStr::from_bytes_with_nul_unchecked(path.as_bytes_with_nul()) }; + rustix::fs::unlinkat(dirfd, path, flags) +} + /// Writes the absolute pathname of the current working directory into `buf`. /// /// The returned C string borrows `buf`, starts at the same address as `buf`,