Skip to content

Commit fe55fb0

Browse files
wan9chiclaude
andcommitted
refactor(fspy-shm): move path ownership to fspy_shared
fspy_shm no longer generates backing paths or owns their lifetime: create takes the path, remove is public, and ShmKeeper/id() are gone. The fspy channel now generates the absolute uniquely-named path (converting long paths to verbatim form up front via omnipath, so open and remove never convert again), and holds its own keeper that removes the path on drop. This stages the next step: with paths supplied by the caller, fspy_shm can drop std entirely and take C-string paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c8bf957 commit fe55fb0

9 files changed

Lines changed: 281 additions & 247 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/fspy_shared/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ vt_path = { workspace = true }
2020

2121
[target.'cfg(target_os = "windows")'.dependencies]
2222
bytemuck = { workspace = true }
23+
omnipath = { workspace = true }
2324
winapi = { workspace = true, features = ["std"] }
2425

2526
[dev-dependencies]

crates/fspy_shared/src/ipc/channel/mod.rs

Lines changed: 91 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod shm_io;
44

55
use std::{env::temp_dir, fs::File, io, ops::Deref, path::PathBuf};
66

7-
use fspy_shm::{Mapping, ShmKeeper};
7+
use fspy_shm::Mapping;
88
pub use shm_io::FrameMut;
99
use shm_io::{ShmReader, ShmWriter};
1010
use tracing::debug;
@@ -13,6 +13,14 @@ use wincode::{SchemaRead, SchemaWrite};
1313

1414
use super::IpcStr;
1515

16+
/// Prefix of shared-memory backing file names inside the system temporary
17+
/// directory.
18+
///
19+
/// The files sit directly in the temporary directory. A shared subdirectory
20+
/// would belong to whichever user created it first and block everyone else;
21+
/// uniquely named `0o600` files in the sticky-bit temp directory avoid that.
22+
const SHM_BACKING_PREFIX: &str = "vite-task-fspy-";
23+
1624
/// Serializable configuration to create channel senders.
1725
#[derive(SchemaWrite, SchemaRead, Clone, Debug)]
1826
pub struct ChannelConf {
@@ -26,18 +34,68 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
2634
// Initialize the lock file with a unique name.
2735
let lock_file_path = temp_dir().join(format!("fspy_ipc_{}.lock", Uuid::new_v4()));
2836

29-
let (keeper, handle) = fspy_shm::create(capacity)?;
37+
let shm_path = shm_backing_path()?;
38+
let handle = fspy_shm::create(shm_path.as_os_str(), capacity)?;
39+
// The keeper exists from here on, so every error path below cleans up.
40+
let keeper = ShmKeeper { path: shm_path };
3041
let mapping = handle.map()?;
3142

3243
let conf = ChannelConf {
3344
lock_file_path: lock_file_path.as_os_str().into(),
34-
shm_id: keeper.id().into(),
45+
shm_id: keeper.path.as_os_str().into(),
3546
};
3647

3748
let receiver = Receiver::new(lock_file_path, keeper, mapping)?;
3849
Ok((conf, receiver))
3950
}
4051

52+
/// Returns a fresh absolute path for a shared-memory backing file.
53+
fn shm_backing_path() -> io::Result<PathBuf> {
54+
// `temp_dir` reflects `TMPDIR` verbatim, which may be relative. The path
55+
// travels to processes with other working directories, so resolve it
56+
// against the creator's current directory first.
57+
let path = std::path::absolute(temp_dir())?
58+
.join(format!("{SHM_BACKING_PREFIX}{}.shm", Uuid::new_v4().simple()));
59+
#[cfg(windows)]
60+
let path = to_verbatim_if_long(path)?;
61+
Ok(path)
62+
}
63+
64+
/// Converts long paths to verbatim (`\\?\`) form up front, so every later use
65+
/// of the path — creation here, opening in any process, removal — stays clear
66+
/// of the legacy `MAX_PATH` limit without relying on the system's long-path
67+
/// opt-in, which the arbitrary processes opening shared memory could not
68+
/// count on anyway.
69+
#[cfg(windows)]
70+
fn to_verbatim_if_long(path: PathBuf) -> io::Result<PathBuf> {
71+
use std::os::windows::ffi::OsStrExt as _;
72+
73+
use omnipath::windows::WinPathExt as _;
74+
75+
// The length at which std's own Windows path conversion switches to a
76+
// verbatim path.
77+
const VERBATIM_THRESHOLD: usize = 248;
78+
79+
if path.as_os_str().encode_wide().count() >= VERBATIM_THRESHOLD {
80+
return path.to_verbatim();
81+
}
82+
Ok(path)
83+
}
84+
85+
/// Keeps the shared memory's backing path alive and removes it on drop.
86+
///
87+
/// Removal is cleanup, not a stop signal: later opens fail, but existing
88+
/// handles and mappings keep reading and writing; see [`fspy_shm::remove`].
89+
struct ShmKeeper {
90+
path: PathBuf,
91+
}
92+
93+
impl Drop for ShmKeeper {
94+
fn drop(&mut self) {
95+
let _ = fspy_shm::remove(self.path.as_os_str());
96+
}
97+
}
98+
4199
impl ChannelConf {
42100
/// Creates a sender.
43101
///
@@ -93,7 +151,8 @@ unsafe impl Sync for Sender {}
93151
pub struct Receiver {
94152
lock_file_path: PathBuf,
95153
lock_file: File,
96-
/// Keeps the backing file's name alive for as long as senders may attach.
154+
/// Keeps the shared memory's backing file alive for as long as senders
155+
/// may attach.
97156
_keeper: ShmKeeper,
98157
mapping: Mapping,
99158
}
@@ -156,13 +215,40 @@ impl<'a> Deref for ReceiverLockGuard<'a> {
156215

157216
#[cfg(test)]
158217
mod tests {
159-
use std::{num::NonZeroUsize, str::from_utf8};
218+
use std::{ffi::OsString, fs, num::NonZeroUsize, str::from_utf8};
160219

161220
use bstr::B;
162221
use subprocess_test::command_for_fn;
163222

164223
use super::*;
165224

225+
/// The shared-memory path is generated absolute, so a sender in a process
226+
/// with a different working directory and a relative temporary directory
227+
/// must still attach.
228+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
229+
async fn sender_ignores_changed_temp_and_working_directory() {
230+
let (conf, receiver) = channel(100).unwrap();
231+
let changed_cwd = temp_dir().join(format!("fspy-ipc-changed-cwd-{}", Uuid::new_v4()));
232+
fs::create_dir(&changed_cwd).unwrap();
233+
234+
let mut command = command_for_fn!(conf, |conf: ChannelConf| {
235+
let sender = conf.sender().unwrap();
236+
let frame_size = NonZeroUsize::new(2).unwrap();
237+
let mut frame = sender.claim_frame(frame_size).unwrap();
238+
frame.copy_from_slice(&[4, 2]);
239+
});
240+
command.cwd = changed_cwd.clone();
241+
for name in ["TMPDIR", "TMP", "TEMP"] {
242+
command.envs.insert(OsString::from(name), OsString::from("changed-relative-tmp"));
243+
}
244+
let succeeded = std::process::Command::from(command).status().unwrap().success();
245+
fs::remove_dir(changed_cwd).unwrap();
246+
assert!(succeeded);
247+
248+
let lock = receiver.lock().unwrap();
249+
assert_eq!(lock.iter_frames().next().unwrap(), &[4, 2]);
250+
}
251+
166252
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
167253
async fn smoke() {
168254
let (conf, receiver) = channel(100).unwrap();

crates/fspy_shared/src/ipc/channel/shm_io.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -672,8 +672,10 @@ mod tests {
672672

673673
const SHM_SIZE: usize = 1024 * 1024;
674674

675-
let (keeper, handle) = fspy_shm::create(SHM_SIZE).unwrap();
676-
let shm_name = keeper.id().to_str().expect("test temp dir is UTF-8").to_owned();
675+
let shm_path = crate::ipc::channel::shm_backing_path().unwrap();
676+
let handle = fspy_shm::create(shm_path.as_os_str(), SHM_SIZE).unwrap();
677+
let _keeper = crate::ipc::channel::ShmKeeper { path: shm_path.clone() };
678+
let shm_name = shm_path.to_str().expect("test temp dir is UTF-8").to_owned();
677679
// Map before the children run. Windows keeps views coherent while they
678680
// exist at the same time; a view created after every writer exited can
679681
// observe the file before the writers' dirty pages reach it.

crates/fspy_shm/Cargo.toml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,10 @@ license.workspace = true
77
publish = false
88
rust-version.workspace = true
99

10-
[dependencies]
11-
uuid = { workspace = true, features = ["v4"] }
12-
1310
[target.'cfg(any(unix, windows))'.dependencies]
1411
fspy_nostd = { workspace = true }
1512

1613
[target.'cfg(target_os = "windows")'.dependencies]
17-
omnipath = { workspace = true }
1814
windows-sys = { workspace = true, features = [
1915
"Win32_Foundation",
2016
"Win32_Storage_FileSystem",
@@ -25,6 +21,7 @@ windows-sys = { workspace = true, features = [
2521
[dev-dependencies]
2622
ctor = { workspace = true }
2723
subprocess_test = { workspace = true }
24+
uuid = { workspace = true, features = ["v4"] }
2825

2926
[lints]
3027
workspace = true

crates/fspy_shm/README.md

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,44 @@
11
# `fspy_shm`
22

3-
`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.
3+
`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.
44

5-
`fspy_shm` exposes only the operations used by fspy. Treat an identifier as an opaque `OsStr`; do not depend on how it is built.
5+
`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.
66

77
## API
88

99
The public API is defined in [`src/lib.rs`](src/lib.rs).
1010

1111
| API | Contract |
1212
| --------------------- | --------------------------------------------------------------------------------------- |
13-
| `create(size)` | Creates a zero-initialized backing file and returns its `ShmKeeper` and an `ShmHandle`. |
14-
| `open(id)` | Opens an `ShmHandle` on the shared memory identified by `id`. |
15-
| `ShmKeeper::id()` | Returns the identifier another process passes to `open`. |
13+
| `create(path, size)` | Creates a zero-initialized backing file at `path` and returns an opened `ShmHandle`. |
14+
| `open(path)` | Opens an `ShmHandle` on the shared memory backed by the file at `path`. |
15+
| `remove(path)` | Removes the backing file. Later opens fail; existing handles and mappings keep working. |
1616
| `ShmHandle::map()` | Maps the shared bytes. Callable more than once. |
1717
| `Mapping::len()` | Returns the mapped size. |
1818
| `Mapping::as_ptr()` | Returns a mutable raw pointer to the first byte. |
1919
| `Mapping::as_slice()` | Returns the bytes as a shared slice. The caller must prevent mutation for its lifetime. |
2020

21-
`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.
21+
`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.
2222

2323
Every byte in a mapping returned by `create` is initially zero. `open` exposes the mapping's current contents and does not reinitialize them.
2424

2525
## Implementation
2626

27-
One implementation serves every platform: a sparse file named `vite-task-fspy-<uuid>.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.
27+
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.
2828

2929
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.
3030

31-
Mapping goes through `memmap2` on every platform. The remaining platform-specific parts are three short passages:
31+
Every operation goes through [`fspy_nostd`](../fspy_nostd) wrappers or direct Win32 calls. The platform-specific parts are three short passages:
3232

33-
| Concern | Unix | Windows |
34-
| ----------------- | ---------------------------------------- | --------------------------------------------------------------------------- |
35-
| Same-user access | `mode(0o600)` on the backing file | the per-user `%TEMP%` ACL |
36-
| Sparseness | file holes, produced by setting a length | `FSCTL_SET_SPARSE` before setting a length, or NTFS allocates every cluster |
37-
| Keeper cleanup | unlink the path | unlink the path; see the fallback below |
38-
| Descriptor safety | `O_CLOEXEC`, the Rust standard default | non-inheritable handles, the Rust standard default |
33+
| Concern | Unix | Windows |
34+
| ---------------- | ---------------------------------------- | --------------------------------------------------------------------------- |
35+
| Same-user access | `mode(0o600)` on the backing file | the per-user `%TEMP%` ACL of the caller's chosen directory |
36+
| Sparseness | file holes, produced by setting a length | `FSCTL_SET_SPARSE` before setting a length, or NTFS allocates every cluster |
37+
| Removal | unlink the path | POSIX delete via `FileDispositionInfoEx`; see below |
3938

4039
`FILE_ATTRIBUTE_TEMPORARY` asks Windows to keep the data in memory when it can. Creation fails on a volume without sparse-file support.
4140

42-
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.
41+
`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.
4342

4443
## Options considered
4544

@@ -61,12 +60,10 @@ Earlier revisions rejected temporary files because dirty pages can reach disk. O
6160

6261
## Lifetime semantics
6362

64-
`create` returns the only keeper. `open` returns `ShmHandle`s.
63+
- While the backing file exists, a process that knows the path can open the shared memory.
64+
- `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.
65+
- An `ShmHandle` and its `Mapping`s stay usable after the backing file is removed. They keep the bytes alive and cannot restore the path.
6566

66-
- While the keeper is alive, a process that knows the identifier can open the shared memory.
67-
- 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.
68-
- 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.
67+
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.
6968

70-
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.
71-
72-
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.
69+
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.

0 commit comments

Comments
 (0)