Skip to content

Commit 7ff65af

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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6144412 commit 7ff65af

10 files changed

Lines changed: 70 additions & 50 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: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use allocator_api2::alloc::Global;
12
use fspy_shared::ipc::{
23
PathAccess,
34
channel::{FrameReader, Receiver},
@@ -37,7 +38,7 @@ pub struct ChannelAccesses {
3738
frames: FrameReader,
3839
}
3940

40-
impl TryFrom<Receiver> for ChannelAccesses {
41+
impl TryFrom<Receiver<Global>> for ChannelAccesses {
4142
type Error = TrackingIncomplete;
4243

4344
/// Closes the channel and takes every record it collected.
@@ -52,7 +53,7 @@ impl TryFrom<Receiver> for ChannelAccesses {
5253
/// [`TrackingIncomplete`] when a tracked process could not record
5354
/// something it went on to do. What did arrive is then a subset of
5455
/// what the run really touched, so none of it is handed back.
55-
fn try_from(receiver: Receiver) -> Result<Self, TrackingIncomplete> {
56+
fn try_from(receiver: Receiver<Global>) -> Result<Self, TrackingIncomplete> {
5657
Ok(Self { frames: receiver.close().map_err(|_| TrackingIncomplete)? })
5758
}
5859
}

crates/fspy/src/unix/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ impl SpyImpl {
8080

8181
#[cfg(not(target_env = "musl"))]
8282
let (ipc_channel_conf, ipc_receiver) =
83-
channel(crate::ipc::shm_capacity()).map_err(SpawnError::ChannelCreation)?;
83+
channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global)
84+
.map_err(SpawnError::ChannelCreation)?;
8485

8586
let payload = Payload {
8687
#[cfg(not(target_env = "musl"))]

crates/fspy/src/windows/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ impl SpyImpl {
8484
command.creation_flags(CREATE_SUSPENDED);
8585

8686
let (channel_conf, receiver) =
87-
channel(crate::ipc::shm_capacity()).map_err(SpawnError::ChannelCreation)?;
87+
channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global)
88+
.map_err(SpawnError::ChannelCreation)?;
8889

8990
let mut spawn_success = false;
9091
let spawn_success = &mut spawn_success;

crates/fspy_client_unix/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ impl Client {
5454
// process starts after the root target exited. Nothing is said
5555
// about it: a preload library writing to the traced process's
5656
// stderr corrupts whatever that process is printing.
57-
let ipc_sender = encoded_payload.payload.ipc_channel_conf.sender();
57+
let ipc_sender =
58+
encoded_payload.payload.ipc_channel_conf.sender(fspy_nostd_alloc::pooled_bump());
5859

5960
Self { encoded_payload, ipc_sender }
6061
}

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
@@ -20,7 +20,7 @@ impl<'a> Client<'a> {
2020
// process starts after the root target exited. Nothing is said
2121
// about it: a detours DLL writing to the traced process's stderr
2222
// corrupts whatever that process is printing.
23-
let ipc_sender = payload.channel_conf.sender();
23+
let ipc_sender = payload.channel_conf.sender(fspy_nostd_alloc::pooled_bump());
2424

2525
Self { payload, ipc_sender }
2626
}

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

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ mod shm_io;
99

1010
use std::{env::temp_dir, ffi::OsStr, io, num::NonZeroUsize, path::PathBuf};
1111

12-
use allocator_api2::alloc::Global;
12+
use allocator_api2::alloc::Allocator;
1313
use fspy_nostd::Fat;
1414
use fspy_nostd_alloc::OsCString;
1515
use fspy_shm::Mapping;
@@ -48,8 +48,11 @@ pub struct ChannelConf {
4848

4949
/// Creates a mpsc IPC channel with one receiver and a `ChannelConf` that can be passed around processes and used to create multiple senders.
5050
#[expect(clippy::missing_errors_doc, reason = "non-vt crate: cannot use vt_str/vt_path types")]
51-
pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
52-
let shm_c_path = os_c_string(shm_backing_path()?.as_os_str())?;
51+
pub fn channel<A: Allocator>(
52+
capacity: usize,
53+
allocator: A,
54+
) -> io::Result<(ChannelConf, Receiver<A>)> {
55+
let shm_c_path = os_c_string(shm_backing_path()?.as_os_str(), allocator)?;
5356
let handle =
5457
fspy_shm::create(shm_c_path.as_c_str().as_thin(), capacity).map_err(shm_error_to_io)?;
5558
// The keeper exists from here on, so every error path below cleans up.
@@ -74,27 +77,27 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
7477
}
7578

7679
/// Encodes `path` as an owned NUL-terminated platform C string.
77-
fn os_c_string(path: &OsStr) -> io::Result<OsCString<Fat, Global>> {
78-
let mut units = os_units(path);
80+
fn os_c_string<A: Allocator>(path: &OsStr, allocator: A) -> io::Result<OsCString<Fat, A>> {
81+
let mut units = os_units(path, allocator);
7982
units.push(0);
8083
OsCString::from_vec_with_nul(units)
8184
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "path contains NUL"))
8285
}
8386

8487
#[cfg(unix)]
85-
fn os_units(path: &OsStr) -> allocator_api2::vec::Vec<u8> {
88+
fn os_units<A: Allocator>(path: &OsStr, allocator: A) -> allocator_api2::vec::Vec<u8, A> {
8689
use std::os::unix::ffi::OsStrExt as _;
8790

88-
let mut units = allocator_api2::vec::Vec::with_capacity(path.len() + 1);
91+
let mut units = allocator_api2::vec::Vec::with_capacity_in(path.len() + 1, allocator);
8992
units.extend_from_slice(path.as_bytes());
9093
units
9194
}
9295

9396
#[cfg(windows)]
94-
fn os_units(path: &OsStr) -> allocator_api2::vec::Vec<u16> {
97+
fn os_units<A: Allocator>(path: &OsStr, allocator: A) -> allocator_api2::vec::Vec<u16, A> {
9598
use std::os::windows::ffi::OsStrExt as _;
9699

97-
let mut units = allocator_api2::vec::Vec::with_capacity(path.len() + 1);
100+
let mut units = allocator_api2::vec::Vec::with_capacity_in(path.len() + 1, allocator);
98101
for unit in path.encode_wide() {
99102
units.push(unit);
100103
}
@@ -148,11 +151,11 @@ fn to_verbatim_if_long(path: PathBuf) -> io::Result<PathBuf> {
148151
///
149152
/// Removal is cleanup, not a stop signal: later opens fail, but existing
150153
/// handles and mappings keep reading and writing; see [`fspy_shm::remove`].
151-
struct ShmKeeper {
152-
path: OsCString<Fat, Global>,
154+
struct ShmKeeper<A: Allocator> {
155+
path: OsCString<Fat, A>,
153156
}
154157

155-
impl Drop for ShmKeeper {
158+
impl<A: Allocator> Drop for ShmKeeper<A> {
156159
fn drop(&mut self) {
157160
let _ = fspy_shm::remove(self.path.as_c_str().as_thin());
158161
}
@@ -174,14 +177,12 @@ impl ChannelConf {
174177
/// receiver it recorded nothing, and a trace that silently omits every
175178
/// access a process made is worse than no trace, so it stops here.
176179
#[must_use]
177-
pub fn sender(&self) -> Option<Sender> {
178-
// The arena never touches the process heap, so this stays safe in
179-
// the preload contexts that create senders (pre-`main` constructors,
180-
// the Windows loader lock).
181-
let arena = fspy_nostd_alloc::pooled_bump();
180+
pub fn sender<A: Allocator>(&self, allocator: A) -> Option<Sender> {
181+
// The allocation is transient: the decoded path only has to outlive
182+
// the open call below.
182183
let shm_path = self
183184
.shm_id
184-
.to_os_c_string_in(&arena)
185+
.to_os_c_string_in(allocator)
185186
.expect("the channel's shared-memory path is not a valid C string");
186187
let mapping = match fspy_shm::open(shm_path.as_c_str().as_thin()) {
187188
Ok(handle) => handle.map().expect("cannot map the shared-memory channel"),
@@ -256,23 +257,23 @@ unsafe impl Sync for Sender {}
256257
///
257258
/// Holds the shared memory and its backing file alive for as long as senders
258259
/// may attach; [`Receiver::close`] (or dropping) removes the backing file.
259-
pub struct Receiver {
260+
pub struct Receiver<A: Allocator> {
260261
/// Keeps the shared memory's backing file alive for as long as senders
261262
/// may attach.
262-
_keeper: ShmKeeper,
263+
_keeper: ShmKeeper<A>,
263264
mapping: Mapping,
264265
}
265266

266267
// SAFETY: `Receiver` only holds the mapping; it accesses it exclusively
267268
// through the `shm_io` protocol in `close`, which synchronizes with senders
268269
// via atomic operations. The mapping's address is stable and independently
269270
// owned.
270-
unsafe impl Send for Receiver {}
271+
unsafe impl<A: Allocator + Send> Send for Receiver<A> {}
271272

272273
// SAFETY: see the `Send` impl.
273-
unsafe impl Sync for Receiver {}
274+
unsafe impl<A: Allocator + Sync> Sync for Receiver<A> {}
274275

275-
impl Receiver {
276+
impl<A: Allocator> Receiver<A> {
276277
/// Closes the channel and returns every committed frame, borrowed from
277278
/// the shared mapping that moves into the returned [`FrameReader`].
278279
///
@@ -333,6 +334,7 @@ pub struct RecordsLost;
333334
mod tests {
334335
use std::{ffi::OsString, fs, num::NonZeroUsize, str::from_utf8};
335336

337+
use allocator_api2::alloc::Global;
336338
use assert2::assert;
337339
use bstr::B;
338340
use subprocess_test::command_for_fn;
@@ -353,7 +355,7 @@ mod tests {
353355
fn a_capacity_too_small_for_the_table_fails_the_channel() {
354356
// The counters alone need sixteen bytes, and the table needs eight
355357
// per slot on top.
356-
let Err(error) = channel(8) else {
358+
let Err(error) = channel(8, Global) else {
357359
panic!("a region too small for the protocol made a channel");
358360
};
359361
assert!(error.kind() == io::ErrorKind::InvalidInput);
@@ -364,12 +366,12 @@ mod tests {
364366
/// must still attach.
365367
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
366368
async fn sender_ignores_changed_temp_and_working_directory() {
367-
let (conf, receiver) = channel(CAPACITY).unwrap();
369+
let (conf, receiver) = channel(CAPACITY, Global).unwrap();
368370
let changed_cwd = temp_dir().join(format!("fspy-ipc-changed-cwd-{}", Uuid::new_v4()));
369371
fs::create_dir(&changed_cwd).unwrap();
370372

371373
let mut command = command_for_fn!(conf, |conf: ChannelConf| {
372-
let sender = conf.sender().unwrap();
374+
let sender = conf.sender(Global).unwrap();
373375
let frame_size = NonZeroUsize::new(2).unwrap();
374376
let mut frame = sender.writer.claim_frame(frame_size).unwrap();
375377
frame.copy_from_slice(&[4, 2]);
@@ -394,8 +396,8 @@ mod tests {
394396
/// here rather than in a build.
395397
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
396398
async fn sender_round_trips_records() {
397-
let (conf, receiver) = channel(CAPACITY).unwrap();
398-
let sender = conf.sender().unwrap();
399+
let (conf, receiver) = channel(CAPACITY, Global).unwrap();
400+
let sender = conf.sender(Global).unwrap();
399401
// A record path carries the platform's own string form: bytes on
400402
// unix, UTF-16 on Windows.
401403
#[cfg(unix)]
@@ -425,9 +427,9 @@ mod tests {
425427

426428
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
427429
async fn smoke() {
428-
let (conf, receiver) = channel(CAPACITY).unwrap();
430+
let (conf, receiver) = channel(CAPACITY, Global).unwrap();
429431
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
430-
let sender = conf.sender().unwrap();
432+
let sender = conf.sender(Global).unwrap();
431433
let frame_size = NonZeroUsize::new(2).unwrap();
432434
let mut frame = sender.writer.claim_frame(frame_size).unwrap();
433435
frame.copy_from_slice(&[4, 2]);
@@ -447,11 +449,11 @@ mod tests {
447449
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
448450
#[expect(clippy::print_stdout, reason = "test diagnostics")]
449451
async fn forbid_new_senders_after_close() {
450-
let (conf, receiver) = channel(CAPACITY).unwrap();
452+
let (conf, receiver) = channel(CAPACITY, Global).unwrap();
451453
let _frames = receiver.close().unwrap();
452454

453455
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
454-
print!("{}", conf.sender().is_some());
456+
print!("{}", conf.sender(Global).is_some());
455457
});
456458
let output = std::process::Command::from(cmd).output().unwrap();
457459
assert!(B(&output.stdout) == B("false"));
@@ -460,11 +462,11 @@ mod tests {
460462
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
461463
#[expect(clippy::print_stdout, reason = "test diagnostics")]
462464
async fn forbid_new_senders_after_receiver_dropped() {
463-
let (conf, receiver) = channel(CAPACITY).unwrap();
465+
let (conf, receiver) = channel(CAPACITY, Global).unwrap();
464466
drop(receiver);
465467

466468
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
467-
print!("{}", conf.sender().is_some());
469+
print!("{}", conf.sender(Global).is_some());
468470
});
469471
let output = std::process::Command::from(cmd).output().unwrap();
470472
assert!(B(&output.stdout) == B("false"));
@@ -474,8 +476,8 @@ mod tests {
474476
/// claim any new frame afterwards.
475477
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
476478
async fn attached_sender_cannot_claim_after_close() {
477-
let (conf, receiver) = channel(CAPACITY).unwrap();
478-
let sender = conf.sender().unwrap();
479+
let (conf, receiver) = channel(CAPACITY, Global).unwrap();
480+
let sender = conf.sender(Global).unwrap();
479481

480482
let mut frame = sender.writer.claim_frame(NonZeroUsize::new(2).unwrap()).unwrap();
481483
frame.copy_from_slice(&[4, 2]);
@@ -492,10 +494,10 @@ mod tests {
492494

493495
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
494496
async fn concurrent_senders() {
495-
let (conf, receiver) = channel(CAPACITY).unwrap();
497+
let (conf, receiver) = channel(CAPACITY, Global).unwrap();
496498
for i in 0u16..200 {
497499
let cmd = command_for_fn!((conf.clone(), i), |(conf, i): (ChannelConf, u16)| {
498-
let sender = conf.sender().unwrap();
500+
let sender = conf.sender(Global).unwrap();
499501
let data_to_send = i.to_string();
500502
let mut frame = sender
501503
.writer

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,9 @@ mod tests {
567567

568568
let shm_path = crate::ipc::channel::shm_backing_path().unwrap();
569569
let shm_name = shm_path.to_str().expect("test temp dir is UTF-8").to_owned();
570-
let c_path = crate::ipc::channel::os_c_string(shm_path.as_os_str()).unwrap();
570+
let c_path =
571+
crate::ipc::channel::os_c_string(shm_path.as_os_str(), allocator_api2::alloc::Global)
572+
.unwrap();
571573
let handle = fspy_shm::create(c_path.as_c_str().as_thin(), SHM_SIZE).unwrap();
572574
let _keeper = crate::ipc::channel::ShmKeeper { path: c_path };
573575
// Map before the children run. Windows keeps views coherent while they
@@ -580,9 +582,11 @@ mod tests {
580582
let cmd = command_for_fn!(
581583
(shm_name.clone(), child_index),
582584
|(shm_name, child_index): (String, usize)| {
583-
let c_path =
584-
crate::ipc::channel::os_c_string(std::ffi::OsStr::new(&shm_name))
585-
.unwrap();
585+
let c_path = crate::ipc::channel::os_c_string(
586+
std::ffi::OsStr::new(&shm_name),
587+
allocator_api2::alloc::Global,
588+
)
589+
.unwrap();
586590
let mapping =
587591
fspy_shm::open(c_path.as_c_str().as_thin()).unwrap().map().unwrap();
588592
// SAFETY: `mapping` is a freshly mapped shared memory
@@ -634,13 +638,19 @@ mod tests {
634638

635639
let shm_path = crate::ipc::channel::shm_backing_path().unwrap();
636640
let shm_name = shm_path.to_str().expect("test temp dir is UTF-8").to_owned();
637-
let c_path = crate::ipc::channel::os_c_string(shm_path.as_os_str()).unwrap();
641+
let c_path =
642+
crate::ipc::channel::os_c_string(shm_path.as_os_str(), allocator_api2::alloc::Global)
643+
.unwrap();
638644
let handle = fspy_shm::create(c_path.as_c_str().as_thin(), SHM_SIZE).unwrap();
639645
let _keeper = crate::ipc::channel::ShmKeeper { path: c_path };
640646
let mapping = handle.map().unwrap();
641647

642648
let cmd = command_for_fn!(shm_name, |shm_name: String| {
643-
let c_path = crate::ipc::channel::os_c_string(std::ffi::OsStr::new(&shm_name)).unwrap();
649+
let c_path = crate::ipc::channel::os_c_string(
650+
std::ffi::OsStr::new(&shm_name),
651+
allocator_api2::alloc::Global,
652+
)
653+
.unwrap();
644654
let child_mapping = fspy_shm::open(c_path.as_c_str().as_thin()).unwrap().map().unwrap();
645655
// SAFETY: see `real_shm_across_processes`.
646656
let writer = unsafe { ShmWriter::new(child_mapping, S) }.unwrap();

0 commit comments

Comments
 (0)