Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions crates/fspy_shared/src/ipc/channel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ pub struct ChannelConf {
lock_file_path: Box<NativeStr>,
#[wincode(with = "ArcStrSchema")]
shm_id: Arc<str>,
shm_size: usize,
}

/// Creates a mpsc IPC channel with one receiver and a `ChannelConf` that can be passed around processes and used to create multiple senders
Expand All @@ -65,11 +64,8 @@ pub fn channel(capacity: usize) -> io::Result<(ChannelConf, Receiver)> {

let shm = fspy_shm::create(capacity)?;

let conf = ChannelConf {
lock_file_path: lock_file_path.as_os_str().into(),
shm_id: shm.id().into(),
shm_size: capacity,
};
let conf =
ChannelConf { lock_file_path: lock_file_path.as_os_str().into(), shm_id: shm.id().into() };

let receiver = Receiver::new(lock_file_path, shm)?;
Ok((conf, receiver))
Expand All @@ -87,7 +83,7 @@ impl ChannelConf {
let lock_file = File::open(self.lock_file_path.to_cow_os_str())?;
lock_file.try_lock_shared()?;

let shm = fspy_shm::open(&self.shm_id, self.shm_size)?;
let shm = fspy_shm::open(&self.shm_id)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the original channel capacity when opening senders

On Windows, opening without the original capacity lets shared_memory recompute the opened view length from the OS mapping, which is page-rounded, while the receiver's owner still reports the requested channel(capacity) length. For non-page-aligned capacities, a sender can therefore use a larger shm.len() for ShmWriter bounds checks and advance the shared header past the receiver's slice, so ReceiverLockGuard::iter_frames can panic when it slices content[..content_size] after enough writes. Keep the configured size in ChannelConf or clamp opened views to the requested capacity.

Useful? React with 👍 / 👎.

// SAFETY: `shm` is a freshly opened shared memory region with valid pointer and size.
// Exclusive write access is ensured by the shared file lock held by this sender.
let writer = unsafe { ShmWriter::new(shm) };
Expand Down
2 changes: 1 addition & 1 deletion crates/fspy_shared/src/ipc/channel/shm_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ mod tests {
let cmd = command_for_fn!(
(shm_name.clone(), child_index),
|(shm_name, child_index): (String, usize)| {
let shm = fspy_shm::open(&shm_name, SHM_SIZE).unwrap();
let shm = fspy_shm::open(&shm_name).unwrap();
// SAFETY: `shm` is a freshly opened shared memory region with a valid
// pointer and size. Concurrent write access is safe because `ShmWriter`
// uses atomic operations.
Expand Down
34 changes: 34 additions & 0 deletions crates/fspy_shm/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# `fspy_shm`

`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.

`fspy_shm` exposes only the operations used by fspy. Callers must treat an identifier as a string and must not depend on a platform's naming scheme.

## API

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

| API | Contract |
| ----------------- | ---------------------------------------------------------------------------------- |
| `create(size)` | Creates a non-empty mapping and returns its unique owner. |
| `open(id)` | Opens another view of the mapping identified by `id`. |
| `Shm::id()` | Returns the identifier to send to another process. |
| `Shm::len()` | Returns the mapped size. |
| `Shm::as_ptr()` | Returns a mutable raw pointer to the first byte. |
| `Shm::as_slice()` | Returns a shared slice. The caller must prevent mutation for the slice's lifetime. |

`Shm` does not synchronize memory access. The fspy channel combines it with atomic frame headers and a lock file. Senders hold a shared file lock while writing. The receiver takes the exclusive lock before reading, which waits for existing senders and rejects new ones.

## Ownership semantics

`create` returns the only owner. `open` returns non-owning views.

- While the owner is alive, a process that knows the identifier can open the mapping.
- An opened view remains usable after the owner is dropped. Its operating system mapping keeps the underlying bytes alive.
- After the owner is dropped, new opens behave differently by platform. POSIX removes the name. Windows can continue accepting opens by section name until the final handle or view is closed.

The channel hides that difference with its lock file. [`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`. The receiver removes that path before dropping the owner, so a sender that starts later fails before opening shared memory.

## Backend boundary

At this point in the stack, `fspy_shm` delegates mapping creation and opening to the [`shared_memory`](https://crates.io/crates/shared_memory) crate. Because callers use only the API above, later changes can replace the backend on each platform without changing the channel protocol.
28 changes: 18 additions & 10 deletions crates/fspy_shm/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Behavior-neutral shared-memory facade for fspy channels.
#![doc = include_str!("../README.md")]

use std::io;

Expand All @@ -9,7 +9,11 @@ pub struct Shm {
inner: Shmem,
}

/// Creates a shared-memory mapping of `size` bytes.
/// Creates a shared-memory mapping of `size` bytes and returns its owner.
///
/// Dropping the returned owner stops new [`open`] calls from being guaranteed
/// to succeed, while views that are already open stay usable (see the
/// [ownership semantics](crate)).
///
/// # Errors
///
Expand All @@ -23,13 +27,17 @@ pub fn create(size: usize) -> io::Result<Shm> {
Ok(Shm { inner })
}

/// Opens the shared-memory mapping identified by `id`.
/// Opens a view of the shared-memory mapping identified by `id`.
///
/// Guaranteed to succeed only while the mapping's owner is alive; the
/// returned view stays usable independently of the owner afterwards (see the
/// [ownership semantics](crate)).
///
/// # Errors
///
/// Returns an error if the mapping does not exist or cannot be mapped.
pub fn open(id: &str, size: usize) -> io::Result<Shm> {
let conf = ShmemConf::new().size(size).os_id(id);
pub fn open(id: &str) -> io::Result<Shm> {
let conf = ShmemConf::new().os_id(id);
#[cfg(target_os = "windows")]
let conf = conf.allow_raw(true);

Expand Down Expand Up @@ -89,7 +97,7 @@ mod tests {
// SAFETY: No writes occur while this slice is borrowed.
assert!(unsafe { owner.as_slice() }.iter().all(|byte| *byte == 0));

let opened = open(owner.id(), SIZE).unwrap();
let opened = open(owner.id()).unwrap();
assert_eq!(opened.id(), owner.id());
assert_eq!(opened.len(), SIZE);

Expand All @@ -105,7 +113,7 @@ mod tests {
write_byte(&owner, 0, 17);

let command = command_for_fn!(owner.id().to_owned(), |id: String| {
let opened = open(&id, SIZE).unwrap();
let opened = open(&id).unwrap();
assert_eq!(read_byte(&opened, 0), 17);
write_byte(&opened, SIZE - 1, 29);
});
Expand All @@ -119,20 +127,20 @@ mod tests {
let id = owner.id().to_owned();
drop(owner);

assert!(open(&id, SIZE).is_err());
assert!(open(&id).is_err());
}

#[test]
fn opened_mapping_survives_owner_drop() {
let owner = create(SIZE).unwrap();
let id = owner.id().to_owned();
let opened = open(&id, SIZE).unwrap();
let opened = open(&id).unwrap();
write_byte(&owner, 0, 17);
drop(owner);

// Windows keeps the named object alive while an opened view exists.
#[cfg(not(target_os = "windows"))]
assert!(open(&id, SIZE).is_err());
assert!(open(&id).is_err());
assert_eq!(read_byte(&opened, 0), 17);
write_byte(&opened, SIZE - 1, 29);
assert_eq!(read_byte(&opened, SIZE - 1), 29);
Expand Down
Loading