Skip to content

Commit a1a77c5

Browse files
wan9chiclaude
andcommitted
refactor(fspy-shared): make the channel allocator-generic
channel(), Receiver, and sender() are now generic over an allocator-api2 allocator instead of hardcoding the global allocator and an internal pooled bump: - channel() threads the caller's allocator through the shared-memory backing path and the ShmKeeper; the supervisor instantiates with Global. - sender() takes the allocator for its transient shm-path decode from the caller, so the choice of preload-safe memory — the preloads pass a pooled bump — lives at the call site instead of inside fspy_shared. - The lock file path is now generated absolute: temp_dir() reflects TMPDIR verbatim, which may be relative, and the path travels to processes with other working directories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a21bce3 commit a1a77c5

10 files changed

Lines changed: 69 additions & 51 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/fspy/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ license.workspace = true
66
publish = false
77

88
[dependencies]
9+
allocator-api2 = { workspace = true, features = ["alloc"] }
910
wincode = { workspace = true }
1011
bstr = { workspace = true, features = ["alloc", "std"] }
1112
bumpalo = { workspace = true }

crates/fspy/src/ipc.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::io;
22

3+
use allocator_api2::alloc::Global;
34
use fspy_shared::ipc::{
45
PathAccess,
56
channel::{Receiver, ReceiverLockGuard},
@@ -14,19 +15,19 @@ pub const SHM_CAPACITY: usize = 4 * 1024 * 1024 * 1024;
1415
#[ouroboros::self_referencing]
1516
pub struct OwnedReceiverLockGuard {
1617
/// Owns the shared memory
17-
receiver: Receiver,
18+
receiver: Receiver<Global>,
1819
/// Borrows the shared memory and owns the file lock
1920
#[borrows(receiver)]
2021
#[covariant]
2122
lock_guard: ReceiverLockGuard<'this>,
2223
}
2324

2425
impl OwnedReceiverLockGuard {
25-
pub fn lock(receiver: Receiver) -> io::Result<Self> {
26+
pub fn lock(receiver: Receiver<Global>) -> io::Result<Self> {
2627
Self::try_new(receiver, fspy_shared::ipc::channel::Receiver::lock)
2728
}
2829

29-
pub async fn lock_async(receiver: Receiver) -> io::Result<Self> {
30+
pub async fn lock_async(receiver: Receiver<Global>) -> io::Result<Self> {
3031
spawn_blocking(move || Self::lock(receiver)).await.expect("lock task panicked")
3132
}
3233

crates/fspy/src/unix/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ impl SpyImpl {
7979
let supervisor = supervise::<SyscallHandler>().map_err(SpawnError::Supervisor)?;
8080

8181
#[cfg(not(target_env = "musl"))]
82-
let (ipc_channel_conf, ipc_receiver) =
83-
channel(SHM_CAPACITY).map_err(SpawnError::ChannelCreation)?;
82+
let (ipc_channel_conf, ipc_receiver) = channel(SHM_CAPACITY, allocator_api2::alloc::Global)
83+
.map_err(SpawnError::ChannelCreation)?;
8484

8585
let payload = Payload {
8686
#[cfg(not(target_env = "musl"))]

crates/fspy/src/windows/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ impl SpyImpl {
8686

8787
command.creation_flags(CREATE_SUSPENDED);
8888

89-
let (channel_conf, receiver) =
90-
channel(SHM_CAPACITY).map_err(SpawnError::ChannelCreation)?;
89+
let (channel_conf, receiver) = channel(SHM_CAPACITY, allocator_api2::alloc::Global)
90+
.map_err(SpawnError::ChannelCreation)?;
9191

9292
let mut spawn_success = false;
9393
let spawn_success = &mut spawn_success;

crates/fspy_client_unix/src/lib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ impl Client {
5353
pub fn from_env(envs: impl Iterator<Item = fspy_nostd::env::Entry>) -> Self {
5454
let encoded_payload = decode_payload_from_env(envs).unwrap();
5555

56-
let ipc_sender = match encoded_payload.payload.ipc_channel_conf.sender() {
56+
let ipc_sender = match encoded_payload
57+
.payload
58+
.ipc_channel_conf
59+
.sender(fspy_nostd_alloc::pooled_bump())
60+
{
5761
Ok(sender) => Some(sender),
5862
Err(err) => {
5963
// This can happen if the process starts after the root target

crates/fspy_preload_windows/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ wincode = { workspace = true }
1313
constcat = { workspace = true }
1414
fspy_detours_sys = { workspace = true }
1515
fspy_nostd = { workspace = true }
16+
fspy_nostd_alloc = { workspace = true }
1617
fspy_shared = { workspace = true }
1718
ntapi = { workspace = true }
1819
smallvec = { workspace = true }

crates/fspy_preload_windows/src/windows/client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ impl<'a> Client<'a> {
1616
pub fn from_payload_bytes(payload_bytes: &'a [u8]) -> Self {
1717
let payload: Payload<'a> = wincode::deserialize_exact(payload_bytes).unwrap();
1818

19-
let ipc_sender = match payload.channel_conf.sender() {
19+
let ipc_sender = match payload.channel_conf.sender(fspy_nostd_alloc::pooled_bump()) {
2020
Ok(sender) => Some(sender),
2121
Err(err) => {
2222
// this can happen if the process is started after the root target process has exited.

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

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

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

7-
use allocator_api2::alloc::Global;
7+
use allocator_api2::alloc::Allocator;
88
use fspy_nostd::Fat;
99
use fspy_nostd_alloc::OsCString;
1010
use fspy_shm::Mapping;
@@ -33,11 +33,17 @@ pub struct ChannelConf {
3333

3434
/// Creates a mpsc IPC channel with one receiver and a `ChannelConf` that can be passed around processes and used to create multiple senders
3535
#[expect(clippy::missing_errors_doc, reason = "non-vt crate: cannot use vt_str/vt_path types")]
36-
pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
37-
// Initialize the lock file with a unique name.
38-
let lock_file_path = temp_dir().join(format!("fspy_ipc_{}.lock", Uuid::new_v4()));
39-
40-
let shm_c_path = os_c_string(shm_backing_path()?.as_os_str())?;
36+
pub fn channel<A: Allocator>(
37+
capacity: usize,
38+
allocator: A,
39+
) -> io::Result<(ChannelConf, Receiver<A>)> {
40+
// Initialize the lock file with a unique name. `temp_dir` reflects
41+
// `TMPDIR` verbatim, which may be relative; the path travels to processes
42+
// with other working directories, so resolve it first.
43+
let lock_file_path =
44+
std::path::absolute(temp_dir())?.join(format!("fspy_ipc_{}.lock", Uuid::new_v4()));
45+
46+
let shm_c_path = os_c_string(shm_backing_path()?.as_os_str(), allocator)?;
4147
let handle =
4248
fspy_shm::create(shm_c_path.as_c_str().as_thin(), capacity).map_err(shm_error_to_io)?;
4349
// The keeper exists from here on, so every error path below cleans up.
@@ -54,27 +60,27 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
5460
}
5561

5662
/// Encodes `path` as an owned NUL-terminated platform C string.
57-
fn os_c_string(path: &OsStr) -> io::Result<OsCString<Fat, Global>> {
58-
let mut units = os_units(path);
63+
fn os_c_string<A: Allocator>(path: &OsStr, allocator: A) -> io::Result<OsCString<Fat, A>> {
64+
let mut units = os_units(path, allocator);
5965
units.push(0);
6066
OsCString::from_vec_with_nul(units)
6167
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "path contains NUL"))
6268
}
6369

6470
#[cfg(unix)]
65-
fn os_units(path: &OsStr) -> allocator_api2::vec::Vec<u8> {
71+
fn os_units<A: Allocator>(path: &OsStr, allocator: A) -> allocator_api2::vec::Vec<u8, A> {
6672
use std::os::unix::ffi::OsStrExt as _;
6773

68-
let mut units = allocator_api2::vec::Vec::with_capacity(path.len() + 1);
74+
let mut units = allocator_api2::vec::Vec::with_capacity_in(path.len() + 1, allocator);
6975
units.extend_from_slice(path.as_bytes());
7076
units
7177
}
7278

7379
#[cfg(windows)]
74-
fn os_units(path: &OsStr) -> allocator_api2::vec::Vec<u16> {
80+
fn os_units<A: Allocator>(path: &OsStr, allocator: A) -> allocator_api2::vec::Vec<u16, A> {
7581
use std::os::windows::ffi::OsStrExt as _;
7682

77-
let mut units = allocator_api2::vec::Vec::with_capacity(path.len() + 1);
83+
let mut units = allocator_api2::vec::Vec::with_capacity_in(path.len() + 1, allocator);
7884
for unit in path.encode_wide() {
7985
units.push(unit);
8086
}
@@ -128,11 +134,11 @@ fn to_verbatim_if_long(path: PathBuf) -> io::Result<PathBuf> {
128134
///
129135
/// Removal is cleanup, not a stop signal: later opens fail, but existing
130136
/// handles and mappings keep reading and writing; see [`fspy_shm::remove`].
131-
struct ShmKeeper {
132-
path: OsCString<Fat, Global>,
137+
struct ShmKeeper<A: Allocator> {
138+
path: OsCString<Fat, A>,
133139
}
134140

135-
impl Drop for ShmKeeper {
141+
impl<A: Allocator> Drop for ShmKeeper<A> {
136142
fn drop(&mut self) {
137143
let _ = fspy_shm::remove(self.path.as_c_str().as_thin());
138144
}
@@ -146,15 +152,13 @@ impl ChannelConf {
146152
clippy::missing_errors_doc,
147153
reason = "error conditions are self-evident from return type"
148154
)]
149-
pub fn sender(&self) -> io::Result<Sender> {
155+
pub fn sender<A: Allocator>(&self, allocator: A) -> io::Result<Sender> {
150156
let lock_file = File::open(self.lock_file_path.to_cow_os_str())?;
151157
lock_file.try_lock_shared()?;
152158

153-
// The arena never touches the process heap, so this stays safe in
154-
// the preload contexts that create senders (pre-`main` constructors,
155-
// the Windows loader lock).
156-
let arena = fspy_nostd_alloc::pooled_bump();
157-
let shm_path = self.shm_id.to_os_c_string_in(&arena).ok_or_else(|| {
159+
// The allocation is transient: the decoded path only has to outlive
160+
// the open call below.
161+
let shm_path = self.shm_id.to_os_c_string_in(allocator).ok_or_else(|| {
158162
io::Error::new(io::ErrorKind::InvalidData, "invalid shared-memory path")
159163
})?;
160164
let mapping = fspy_shm::open(shm_path.as_c_str().as_thin())
@@ -200,31 +204,31 @@ unsafe impl Sync for Sender {}
200204

201205
/// The unique receiver side of an IPC channel.
202206
/// Owns the lock file and removes it on drop.
203-
pub struct Receiver {
207+
pub struct Receiver<A: Allocator> {
204208
lock_file_path: PathBuf,
205209
lock_file: File,
206210
/// Keeps the shared memory's backing file alive for as long as senders
207211
/// may attach.
208-
_keeper: ShmKeeper,
212+
_keeper: ShmKeeper<A>,
209213
mapping: Mapping,
210214
}
211215

212216
/// SAFETY: `Receiver` doesn't read or write `shm`. It only passes it to `ReceiverLockGuard` under the lock.
213-
unsafe impl Send for Receiver {}
217+
unsafe impl<A: Allocator + Send> Send for Receiver<A> {}
214218

215219
/// SAFETY: `Receiver` doesn't read or write `shm`. It only passes it to `ReceiverLockGuard` under the lock.
216-
unsafe impl Sync for Receiver {}
220+
unsafe impl<A: Allocator + Sync> Sync for Receiver<A> {}
217221

218-
impl Drop for Receiver {
222+
impl<A: Allocator> Drop for Receiver<A> {
219223
fn drop(&mut self) {
220224
if let Err(err) = std::fs::remove_file(&self.lock_file_path) {
221225
debug!("Failed to remove IPC lock file {}: {}", self.lock_file_path.display(), err);
222226
}
223227
}
224228
}
225229

226-
impl Receiver {
227-
fn new(lock_file_path: PathBuf, keeper: ShmKeeper, mapping: Mapping) -> io::Result<Self> {
230+
impl<A: Allocator> Receiver<A> {
231+
fn new(lock_file_path: PathBuf, keeper: ShmKeeper<A>, mapping: Mapping) -> io::Result<Self> {
228232
let lock_file = File::create(&lock_file_path)?;
229233
Ok(Self { lock_file_path, lock_file, _keeper: keeper, mapping })
230234
}
@@ -269,6 +273,7 @@ impl<'a> Deref for ReceiverLockGuard<'a> {
269273
mod tests {
270274
use std::{ffi::OsString, fs, num::NonZeroUsize, str::from_utf8};
271275

276+
use allocator_api2::alloc::Global;
272277
use bstr::B;
273278
use subprocess_test::command_for_fn;
274279

@@ -279,12 +284,12 @@ mod tests {
279284
/// must still attach.
280285
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
281286
async fn sender_ignores_changed_temp_and_working_directory() {
282-
let (conf, receiver) = channel(100).unwrap();
287+
let (conf, receiver) = channel(100, Global).unwrap();
283288
let changed_cwd = temp_dir().join(format!("fspy-ipc-changed-cwd-{}", Uuid::new_v4()));
284289
fs::create_dir(&changed_cwd).unwrap();
285290

286291
let mut command = command_for_fn!(conf, |conf: ChannelConf| {
287-
let sender = conf.sender().unwrap();
292+
let sender = conf.sender(Global).unwrap();
288293
let frame_size = NonZeroUsize::new(2).unwrap();
289294
let mut frame = sender.claim_frame(frame_size).unwrap();
290295
frame.copy_from_slice(&[4, 2]);
@@ -303,9 +308,9 @@ mod tests {
303308

304309
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
305310
async fn smoke() {
306-
let (conf, receiver) = channel(100).unwrap();
311+
let (conf, receiver) = channel(100, Global).unwrap();
307312
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
308-
let sender = conf.sender().unwrap();
313+
let sender = conf.sender(Global).unwrap();
309314
let frame_size = NonZeroUsize::new(2).unwrap();
310315
let mut frame = sender.claim_frame(frame_size).unwrap();
311316
frame.copy_from_slice(&[4, 2]);
@@ -324,11 +329,11 @@ mod tests {
324329
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
325330
#[expect(clippy::print_stdout, reason = "test diagnostics")]
326331
async fn forbid_new_senders_after_locked() {
327-
let (conf, receiver) = channel(42).unwrap();
332+
let (conf, receiver) = channel(42, Global).unwrap();
328333
let _lock = receiver.lock().unwrap();
329334

330335
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
331-
print!("{}", conf.sender().is_ok());
336+
print!("{}", conf.sender(Global).is_ok());
332337
});
333338
let output = std::process::Command::from(cmd).output().unwrap();
334339
assert_eq!(B(&output.stdout), B("false"));
@@ -337,22 +342,22 @@ mod tests {
337342
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
338343
#[expect(clippy::print_stdout, reason = "test diagnostics")]
339344
async fn forbid_new_senders_after_receiver_dropped() {
340-
let (conf, receiver) = channel(42).unwrap();
345+
let (conf, receiver) = channel(42, Global).unwrap();
341346
drop(receiver);
342347

343348
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
344-
print!("{}", conf.sender().is_ok());
349+
print!("{}", conf.sender(Global).is_ok());
345350
});
346351
let output = std::process::Command::from(cmd).output().unwrap();
347352
assert_eq!(B(&output.stdout), B("false"));
348353
}
349354

350355
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
351356
async fn concurrent_senders() {
352-
let (conf, receiver) = channel(8192).unwrap();
357+
let (conf, receiver) = channel(8192, Global).unwrap();
353358
for i in 0u16..200 {
354359
let cmd = command_for_fn!((conf.clone(), i), |(conf, i): (ChannelConf, u16)| {
355-
let sender = conf.sender().unwrap();
360+
let sender = conf.sender(Global).unwrap();
356361
let data_to_send = i.to_string();
357362
sender
358363
.claim_frame(NonZeroUsize::new(data_to_send.len()).unwrap())

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -674,7 +674,9 @@ mod tests {
674674

675675
let shm_path = crate::ipc::channel::shm_backing_path().unwrap();
676676
let shm_name = shm_path.to_str().expect("test temp dir is UTF-8").to_owned();
677-
let c_path = crate::ipc::channel::os_c_string(shm_path.as_os_str()).unwrap();
677+
let c_path =
678+
crate::ipc::channel::os_c_string(shm_path.as_os_str(), allocator_api2::alloc::Global)
679+
.unwrap();
678680
let handle = fspy_shm::create(c_path.as_c_str().as_thin(), SHM_SIZE).unwrap();
679681
let _keeper = crate::ipc::channel::ShmKeeper { path: c_path };
680682
// Map before the children run. Windows keeps views coherent while they
@@ -687,9 +689,11 @@ mod tests {
687689
let cmd = command_for_fn!(
688690
(shm_name.clone(), child_index),
689691
|(shm_name, child_index): (String, usize)| {
690-
let c_path =
691-
crate::ipc::channel::os_c_string(std::ffi::OsStr::new(&shm_name))
692-
.unwrap();
692+
let c_path = crate::ipc::channel::os_c_string(
693+
std::ffi::OsStr::new(&shm_name),
694+
allocator_api2::alloc::Global,
695+
)
696+
.unwrap();
693697
let mapping =
694698
fspy_shm::open(c_path.as_c_str().as_thin()).unwrap().map().unwrap();
695699
// SAFETY: `mapping` is a freshly mapped shared memory region with a

0 commit comments

Comments
 (0)