Skip to content

Commit 8716c77

Browse files
wan9chiGPT-5.6
andcommitted
refactor(fspy): add shared-memory service lifecycle
Co-authored-by: GPT-5.6 <gpt-5.6@openai.com>
1 parent cb580c2 commit 8716c77

6 files changed

Lines changed: 172 additions & 20 deletions

File tree

crates/fspy/src/unix/mod.rs

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ use std::{io, path::Path};
1010
use fspy_seccomp_unotify::supervisor::supervise;
1111
use fspy_shared::ipc::PathAccess;
1212
#[cfg(not(target_env = "musl"))]
13-
use fspy_shared::ipc::{NativeStr, channel::channel};
13+
use fspy_shared::ipc::{
14+
NativeStr,
15+
channel::{CreatedChannel, channel},
16+
};
1417
#[cfg(target_os = "macos")]
1518
use fspy_shared_unix::payload::Artifacts;
1619
use fspy_shared_unix::{
@@ -28,6 +31,40 @@ use tokio_util::sync::CancellationToken;
2831
use crate::ipc::{OwnedReceiverLockGuard, SHM_CAPACITY};
2932
use crate::{ChildTermination, Command, TrackedChild, arena::PathAccessArena, error::SpawnError};
3033

34+
#[cfg(all(target_os = "linux", not(target_env = "musl")))]
35+
struct AbortOnDropHandle<T> {
36+
handle: Option<tokio::task::JoinHandle<T>>,
37+
}
38+
39+
#[cfg(all(target_os = "linux", not(target_env = "musl")))]
40+
impl<T> AbortOnDropHandle<T> {
41+
const fn new(handle: tokio::task::JoinHandle<T>) -> Self {
42+
Self { handle: Some(handle) }
43+
}
44+
}
45+
46+
#[cfg(all(target_os = "linux", not(target_env = "musl")))]
47+
impl AbortOnDropHandle<io::Result<()>> {
48+
async fn abort_and_wait(mut self) -> io::Result<()> {
49+
let handle = self.handle.take().expect("broker task handle must be present");
50+
handle.abort();
51+
match handle.await {
52+
Ok(result) => result,
53+
Err(error) if error.is_cancelled() => Ok(()),
54+
Err(error) => Err(io::Error::other(error)),
55+
}
56+
}
57+
}
58+
59+
#[cfg(all(target_os = "linux", not(target_env = "musl")))]
60+
impl<T> Drop for AbortOnDropHandle<T> {
61+
fn drop(&mut self) {
62+
if let Some(handle) = &self.handle {
63+
handle.abort();
64+
}
65+
}
66+
}
67+
3168
#[derive(Debug)]
3269
pub struct SpyImpl {
3370
#[cfg(target_os = "macos")]
@@ -78,8 +115,15 @@ impl SpyImpl {
78115
let supervisor = supervise::<SyscallHandler>().map_err(SpawnError::Supervisor)?;
79116

80117
#[cfg(not(target_env = "musl"))]
81-
let (ipc_channel_conf, ipc_receiver) =
82-
channel(SHM_CAPACITY).map_err(SpawnError::ChannelCreation)?;
118+
let CreatedChannel {
119+
conf: ipc_channel_conf,
120+
receiver: ipc_receiver,
121+
#[cfg(target_os = "linux")]
122+
broker: shm_broker,
123+
} = channel(SHM_CAPACITY).map_err(SpawnError::ChannelCreation)?;
124+
125+
#[cfg(all(target_os = "linux", not(target_env = "musl")))]
126+
let shm_broker_handle = AbortOnDropHandle::new(tokio::spawn(shm_broker));
83127

84128
let payload = Payload {
85129
#[cfg(not(target_env = "musl"))]
@@ -146,6 +190,9 @@ impl SpyImpl {
146190
}
147191
};
148192

193+
#[cfg(all(target_os = "linux", not(target_env = "musl")))]
194+
shm_broker_handle.abort_and_wait().await?;
195+
149196
let arenas = std::iter::once(exec_resolve_accesses);
150197
// Stop the supervisor and collect path accesses from it.
151198
#[cfg(target_os = "linux")]

crates/fspy/src/windows/mod.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ use std::{
88

99
use fspy_detours_sys::{DetourCopyPayloadToProcess, DetourUpdateProcessWithDll};
1010
use fspy_shared::{
11-
ipc::{PathAccess, channel::channel},
11+
ipc::{
12+
PathAccess,
13+
channel::{CreatedChannel, channel},
14+
},
1215
windows::{PAYLOAD_ID, Payload},
1316
};
1417
use futures_util::FutureExt;
@@ -78,7 +81,7 @@ impl SpyImpl {
7881

7982
command.creation_flags(CREATE_SUSPENDED);
8083

81-
let (channel_conf, receiver) =
84+
let CreatedChannel { conf: channel_conf, receiver } =
8285
channel(SHM_CAPACITY).map_err(SpawnError::ChannelCreation)?;
8386

8487
let mut spawn_success = false;
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#![cfg(all(target_os = "linux", not(target_env = "musl")))]
2+
3+
use std::{env::current_exe, io, process::Stdio};
4+
5+
use fspy::error::SpawnError;
6+
use fspy_shared::ipc::channel::{CreatedChannel, channel};
7+
use tokio_util::sync::CancellationToken;
8+
9+
#[test_log::test(tokio::test)]
10+
async fn broker_runs_until_aborted() -> anyhow::Result<()> {
11+
let CreatedChannel { broker, .. } = channel(64 * 1024)?;
12+
let handle = tokio::spawn(broker);
13+
tokio::task::yield_now().await;
14+
assert!(!handle.is_finished());
15+
handle.abort();
16+
assert!(handle.await.unwrap_err().is_cancelled());
17+
Ok(())
18+
}
19+
20+
#[test_log::test(tokio::test)]
21+
async fn injection_failure_aborts_broker() -> anyhow::Result<()> {
22+
let mut command = fspy::Command::new(current_exe()?);
23+
command.arg("--help").stdout(Stdio::null()).env("FSPY_PAYLOAD", "conflict");
24+
25+
let result = command.spawn(CancellationToken::new()).await;
26+
assert!(matches!(result, Err(SpawnError::Injection(_))));
27+
Ok(())
28+
}
29+
30+
#[test_log::test(tokio::test)]
31+
async fn spawn_failure_aborts_broker() -> anyhow::Result<()> {
32+
let mut command = fspy::Command::new(current_exe()?);
33+
command.arg("--help").stdout(Stdio::null());
34+
// SAFETY: The closure only constructs an OS error and returns without touching process state.
35+
unsafe {
36+
command.pre_exec(|| Err(io::Error::from_raw_os_error(libc::EINVAL)));
37+
}
38+
39+
let result = command.spawn(CancellationToken::new()).await;
40+
assert!(matches!(result, Err(SpawnError::OsSpawn(_))));
41+
Ok(())
42+
}

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

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

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

7-
use fspy_shm::Shm;
7+
#[cfg(target_os = "linux")]
8+
pub use fspy_shm::ShmBroker;
9+
use fspy_shm::{CreatedShm, Shm};
810
pub use shm_io::FrameMut;
911
use shm_io::{ShmReader, ShmWriter};
1012
use tracing::debug;
@@ -54,16 +56,31 @@ pub struct ChannelConf {
5456
shm_size: usize,
5557
}
5658

59+
/// A newly created IPC channel and its platform service.
60+
pub struct CreatedChannel {
61+
/// The serializable configuration used to create senders.
62+
pub conf: ChannelConf,
63+
/// The receiving side of the channel.
64+
pub receiver: Receiver,
65+
/// The Linux service future that makes the channel mapping available to senders.
66+
#[cfg(target_os = "linux")]
67+
pub broker: ShmBroker,
68+
}
69+
5770
/// Creates a mpsc IPC channel with one receiver and a `ChannelConf` that can be passed around processes and used to create multiple senders
5871
#[expect(
5972
clippy::missing_errors_doc,
6073
reason = "non-vite crate: cannot use vite_str/vite_path types"
6174
)]
62-
pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
75+
pub fn channel(capacity: usize) -> io::Result<CreatedChannel> {
6376
// Initialize the lock file with a unique name.
6477
let lock_file_path = temp_dir().join(format!("fspy_ipc_{}.lock", Uuid::new_v4()));
6578

66-
let shm = fspy_shm::create(capacity)?;
79+
let CreatedShm {
80+
shm,
81+
#[cfg(target_os = "linux")]
82+
broker,
83+
} = fspy_shm::create(capacity)?;
6784

6885
let conf = ChannelConf {
6986
lock_file_path: lock_file_path.as_os_str().into(),
@@ -72,7 +89,12 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {
7289
};
7390

7491
let receiver = Receiver::new(lock_file_path, shm)?;
75-
Ok((conf, receiver))
92+
Ok(CreatedChannel {
93+
conf,
94+
receiver,
95+
#[cfg(target_os = "linux")]
96+
broker,
97+
})
7698
}
7799

78100
impl ChannelConf {
@@ -206,7 +228,9 @@ mod tests {
206228

207229
#[test]
208230
fn smoke() {
209-
let (conf, receiver) = channel(100).unwrap();
231+
let created = channel(100).unwrap();
232+
let conf = created.conf;
233+
let receiver = created.receiver;
210234
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
211235
let sender = conf.sender().unwrap();
212236
let frame_size = NonZeroUsize::new(2).unwrap();
@@ -227,7 +251,9 @@ mod tests {
227251
#[test]
228252
#[expect(clippy::print_stdout, reason = "test diagnostics")]
229253
fn forbid_new_senders_after_locked() {
230-
let (conf, receiver) = channel(42).unwrap();
254+
let created = channel(42).unwrap();
255+
let conf = created.conf;
256+
let receiver = created.receiver;
231257
let _lock = receiver.lock().unwrap();
232258

233259
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
@@ -240,7 +266,9 @@ mod tests {
240266
#[test]
241267
#[expect(clippy::print_stdout, reason = "test diagnostics")]
242268
fn forbid_new_senders_after_receiver_dropped() {
243-
let (conf, receiver) = channel(42).unwrap();
269+
let created = channel(42).unwrap();
270+
let conf = created.conf;
271+
let receiver = created.receiver;
244272
drop(receiver);
245273

246274
let cmd = command_for_fn!(conf, |conf: ChannelConf| {
@@ -252,7 +280,9 @@ mod tests {
252280

253281
#[test]
254282
fn concurrent_senders() {
255-
let (conf, receiver) = channel(8192).unwrap();
283+
let created = channel(8192).unwrap();
284+
let conf = created.conf;
285+
let receiver = created.receiver;
256286
for i in 0u16..200 {
257287
let cmd = command_for_fn!((conf.clone(), i), |(conf, i): (ChannelConf, u16)| {
258288
let sender = conf.sender().unwrap();

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,10 @@ mod tests {
672672

673673
const SHM_SIZE: usize = 1024 * 1024;
674674

675-
let shm = fspy_shm::create(SHM_SIZE).unwrap();
675+
let created = fspy_shm::create(SHM_SIZE).unwrap();
676+
#[cfg(target_os = "linux")]
677+
let _broker = created.broker;
678+
let shm = created.shm;
676679
let shm_name = shm.id().to_owned();
677680

678681
let children: Vec<Child> = (0..CHILD_COUNT)

crates/fspy_shm/src/lib.rs

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//! Behavior-neutral shared-memory facade for fspy channels.
22
33
use std::io;
4+
#[cfg(target_os = "linux")]
5+
use std::{future::Future, pin::Pin};
46

57
use shared_memory::{Shmem, ShmemConf};
68

@@ -9,18 +11,35 @@ pub struct Shm {
911
inner: Shmem,
1012
}
1113

14+
/// A Linux service future that makes a shared-memory mapping available to other processes.
15+
#[cfg(target_os = "linux")]
16+
pub type ShmBroker = Pin<Box<dyn Future<Output = io::Result<()>> + Send + 'static>>;
17+
18+
/// A newly created shared-memory mapping and its platform service.
19+
pub struct CreatedShm {
20+
/// The owned shared-memory mapping.
21+
pub shm: Shm,
22+
/// The service future that makes this mapping available to other processes.
23+
#[cfg(target_os = "linux")]
24+
pub broker: ShmBroker,
25+
}
26+
1227
/// Creates a shared-memory mapping of `size` bytes.
1328
///
1429
/// # Errors
1530
///
1631
/// Returns an error if the platform cannot create or map the region.
17-
pub fn create(size: usize) -> io::Result<Shm> {
32+
pub fn create(size: usize) -> io::Result<CreatedShm> {
1833
let conf = ShmemConf::new().size(size);
1934
#[cfg(target_os = "windows")]
2035
let conf = conf.allow_raw(true);
2136

2237
let inner = conf.create().map_err(io::Error::other)?;
23-
Ok(Shm { inner })
38+
Ok(CreatedShm {
39+
shm: Shm { inner },
40+
#[cfg(target_os = "linux")]
41+
broker: Box::pin(std::future::pending()),
42+
})
2443
}
2544

2645
/// Opens the shared-memory mapping identified by `id`.
@@ -83,7 +102,7 @@ mod tests {
83102

84103
#[test]
85104
fn create_and_open_are_shared() {
86-
let owner = create(SIZE).unwrap();
105+
let owner = create(SIZE).unwrap().shm;
87106
assert_eq!(owner.len(), SIZE);
88107
assert_eq!(owner.as_ptr() as usize % align_of::<usize>(), 0);
89108
// SAFETY: No writes occur while this slice is borrowed.
@@ -101,7 +120,7 @@ mod tests {
101120

102121
#[test]
103122
fn mapping_is_visible_across_processes() {
104-
let owner = create(SIZE).unwrap();
123+
let owner = create(SIZE).unwrap().shm;
105124
write_byte(&owner, 0, 17);
106125

107126
let command = command_for_fn!(owner.id().to_owned(), |id: String| {
@@ -115,7 +134,7 @@ mod tests {
115134

116135
#[test]
117136
fn owner_drop_prevents_new_opens() {
118-
let owner = create(SIZE).unwrap();
137+
let owner = create(SIZE).unwrap().shm;
119138
let id = owner.id().to_owned();
120139
drop(owner);
121140

@@ -124,7 +143,7 @@ mod tests {
124143

125144
#[test]
126145
fn opened_mapping_survives_owner_drop() {
127-
let owner = create(SIZE).unwrap();
146+
let owner = create(SIZE).unwrap().shm;
128147
let id = owner.id().to_owned();
129148
let opened = open(&id, SIZE).unwrap();
130149
write_byte(&owner, 0, 17);
@@ -138,6 +157,14 @@ mod tests {
138157
assert_eq!(read_byte(&opened, SIZE - 1), 29);
139158
}
140159

160+
#[cfg(target_os = "linux")]
161+
#[test]
162+
fn broker_remains_pending() {
163+
let mut broker = create(SIZE).unwrap().broker;
164+
let mut context = std::task::Context::from_waker(std::task::Waker::noop());
165+
assert!(broker.as_mut().poll(&mut context).is_pending());
166+
}
167+
141168
fn read_byte(shm: &Shm, index: usize) -> u8 {
142169
assert!(index < shm.len());
143170
// SAFETY: The index is in bounds and tests synchronize all accesses.

0 commit comments

Comments
 (0)