refactor(fspy-shm): expose explicit backing-file removal

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<Self> {
-        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<Self> {
+        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.

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();
     }
 }

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.

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::<usize>(), 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 {