Skip to content

Commit 10bdd63

Browse files
wan9chiclaude
andcommitted
refactor(fspy-shm): split what the channel reports from what it panics on
The CLOSED gate now means one thing: the region had no room. That is the only failure `shm_io` owns, since the region's size is the only thing it controls. `report_lost_record` goes with that, and the gate loses its second meaning. Everything else a sender can hit is a defect in this crate, so the channel layer panics instead of reporting. `Sender::send` panics when a record's serialized size disagrees with the bytes it writes, and `sender()` panics when the region is there but cannot be opened, mapped, or attached to. A missing backing file stays an error, because it is not a failure at all: the receiver removed it, so it has already stopped collecting and this process is working past the boundary. That distinction is what lets the rest abort. A process that cannot attach has no way to tell the receiver it recorded nothing, and a trace that silently omits every access a process made is worse than a build that stops. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c11a5b3 commit 10bdd63

7 files changed

Lines changed: 74 additions & 85 deletions

File tree

‎crates/fspy_client_unix/src/lib.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,13 @@ impl Client {
5454

5555
let ipc_sender = match encoded_payload.payload.ipc_channel_conf.sender() {
5656
Ok(sender) => Some(sender),
57+
// The only failure `sender` returns is a channel that has
58+
// already closed, which happens when this process starts after
59+
// the root target exited. Everything it does from here is past
60+
// the receiver's boundary, so recording nothing loses nothing.
61+
// Anything worse stops the process inside `sender` instead.
5762
Err(err) => {
58-
// This can happen if the process starts after the root target
59-
// has exited and the receiver has closed the channel.
60-
eprintln!("fspy: failed to create ipc sender: {err}");
63+
eprintln!("fspy: the trace channel has closed: {err}");
6164
None
6265
}
6366
};

‎crates/fspy_preload_windows/src/windows/client.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,18 @@ impl<'a> Client<'a> {
1818

1919
let ipc_sender = match payload.channel_conf.sender() {
2020
Ok(sender) => Some(sender),
21+
// The only failure `sender` returns is a channel that has
22+
// already closed, which happens when this process starts after
23+
// the root target exited. Everything it does from here is past
24+
// the receiver's boundary, so recording nothing loses nothing.
25+
// Anything worse stops the process inside `sender` instead.
2126
Err(err) => {
22-
// this can happen if the process is started after the root target process has exited.
23-
// By that time the channel would have been closed in the receiver side.
24-
// In this case we just leave a message and skip sending any path accesses.
2527
#[expect(
2628
clippy::print_stderr,
2729
reason = "preload library uses stderr for debug diagnostics"
2830
)]
2931
{
30-
eprintln!("fspy: failed to create ipc sender: {err}");
32+
eprintln!("fspy: the trace channel has closed: {err}");
3133
}
3234
None
3335
}

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

Lines changed: 52 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,19 @@ impl Drop for ShmKeeper {
165165
impl ChannelConf {
166166
/// Creates a sender.
167167
///
168-
/// Never blocks. Fails when the receiver has already closed the channel
169-
/// or dropped, because the backing file is removed either way. A close
170-
/// also shuts the region's gate before removing it, so a sender that
171-
/// attaches in that window still finds a closed channel; a receiver
172-
/// that is merely dropped only removes the file, and a removal that
173-
/// fails there leaves the region attachable.
168+
/// Never blocks. Fails only when the channel is already over: the
169+
/// receiver removed the backing file, or it sealed the region before
170+
/// removing it and a sender caught the gate in between. Either way
171+
/// whatever this process does next happens past the receiver's
172+
/// boundary, so skipping its records loses nothing.
173+
///
174+
/// # Panics
175+
///
176+
/// When the channel is there but cannot be attached to: the file
177+
/// refuses to open or map, or it cannot hold the protocol. A process
178+
/// with no writer has no way to tell the receiver it recorded nothing,
179+
/// and a trace that silently omits every access it made is worse than
180+
/// no trace, so it stops here instead.
174181
#[expect(
175182
clippy::missing_errors_doc,
176183
reason = "error conditions are self-evident from return type"
@@ -180,24 +187,30 @@ impl ChannelConf {
180187
// the preload contexts that create senders (pre-`main` constructors,
181188
// the Windows loader lock).
182189
let arena = fspy_nostd_alloc::arena();
183-
let shm_path = self.shm_id.to_os_c_string_in(&arena).ok_or_else(|| {
184-
io::Error::new(io::ErrorKind::InvalidData, "invalid shared-memory path")
185-
})?;
186-
let mapping = fspy_shm::open(shm_path.as_c_str().as_thin())
187-
.map_err(shm_error_to_io)?
188-
.map()
189-
.map_err(shm_error_to_io)?;
190+
let shm_path = self
191+
.shm_id
192+
.to_os_c_string_in(&arena)
193+
.expect("the channel's own shared-memory path is not a valid C string");
194+
let handle = match fspy_shm::open(shm_path.as_c_str().as_thin()) {
195+
Ok(handle) => handle,
196+
Err(error) => {
197+
let error = shm_error_to_io(error);
198+
// The receiver removed the backing file, so it has already
199+
// stopped collecting.
200+
if error.kind() == io::ErrorKind::NotFound {
201+
return Err(error);
202+
}
203+
panic!("cannot open the shared-memory channel: {error}");
204+
}
205+
};
206+
let mapping = handle.map().unwrap_or_else(|error| {
207+
panic!("cannot map the shared-memory channel: {}", shm_error_to_io(error))
208+
});
190209
// SAFETY: `mapping` is a freshly mapped shared memory region created
191210
// zero-initialized by `channel` and accessed only through the
192211
// `shm_io` protocol by every attached process.
193-
let Some(writer) = (unsafe { ShmWriter::new(mapping) }) else {
194-
// A truncated or foreign file fails here — it never panics the
195-
// host process.
196-
return Err(io::Error::new(
197-
io::ErrorKind::InvalidData,
198-
"shared-memory region cannot host the channel",
199-
));
200-
};
212+
let writer = unsafe { ShmWriter::new(mapping) }
213+
.expect("the shared-memory region cannot hold the channel");
201214
if writer.is_closed() {
202215
return Err(io::Error::new(
203216
io::ErrorKind::BrokenPipe,
@@ -215,32 +228,33 @@ pub struct Sender {
215228
impl Sender {
216229
/// Serializes one record into a committed frame.
217230
///
218-
/// A record that cannot be sent is skipped, because that is all a
219-
/// sender inside an intercepted call can do — but never silently. A
220-
/// failed claim already reports itself: for space by setting the CLOSED
221-
/// gate, or as closed, which means the record belongs past the
222-
/// receiver's boundary. The remaining ways to give up are this
223-
/// sender's own, so it reports them, and the seal then refuses to hand
224-
/// back a set of frames that is missing one.
231+
/// A claim the channel refuses is skipped, because that is all a sender
232+
/// inside an intercepted call can do: the channel has closed, so the
233+
/// record belongs past the receiver's boundary, or the region is full
234+
/// and the failed claim already set the CLOSED gate to say so.
235+
///
236+
/// # Panics
237+
///
238+
/// When the record's serialized size disagrees with the bytes it then
239+
/// writes. Nothing the caller passes can cause that, so it is a defect
240+
/// in this crate or its codec, and a trace built on it would be wrong
241+
/// in ways the receiver cannot see.
225242
pub fn send<T: SchemaWrite<DefaultConfig, Src = T>>(&self, value: &T) {
226243
let Ok(serialized_size) = T::serialized_size(value) else {
227-
self.writer.report_lost_record();
228-
return;
244+
panic!("a record cannot report its serialized size");
229245
};
230246
let Ok(Some(frame_size)) = usize::try_from(serialized_size).map(NonZeroUsize::new) else {
231-
self.writer.report_lost_record();
232-
return;
247+
panic!("a record reports a serialized size of {serialized_size} bytes");
233248
};
234249
let Ok(mut frame) = self.writer.claim_frame(frame_size) else {
235250
return;
236251
};
237252
let mut buf: &mut [u8] = &mut frame;
238-
if T::serialize_into(&mut buf, value).is_err() || !buf.is_empty() {
239-
// The frame is abandoned — the receiver ignores its slot — so
240-
// the loss needs reporting on its own.
241-
self.writer.report_lost_record();
242-
return;
243-
}
253+
let written = T::serialize_into(&mut buf, value);
254+
assert!(
255+
written.is_ok() && buf.is_empty(),
256+
"a record wrote fewer bytes than the {serialized_size} it reported"
257+
);
244258
frame.finish();
245259
}
246260
}

‎crates/fspy_shared/src/ipc/channel/shm_io/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ Where the table ends and payloads begin never moves. Claiming needs no retry loo
3636
- **The process died,** mid-claim or mid-fill. The slot stays zero and the receiver ignores it. No cleanup code runs, because none exists.
3737
- **The process abandoned the frame** and kept going. The slot stays zero and the receiver ignores that too, since it cannot tell the two apart.
3838

39-
So the channel asks one thing of its users: **publish a record before performing the action it describes.** A dead writer's missing record then describes an action that never happened, and a record refused after the seal describes one performed after the channel closed. The receiver drops both. A writer that records after acting loses records with nothing said. A writer that abandons a frame and acts anyway calls `ShmWriter::report_lost_record`.
39+
So the channel asks one thing of its users: **publish a record before performing the action it describes.** A dead writer's missing record then describes an action that never happened, and a record refused after the seal describes one performed after the channel closed. The receiver drops both. A writer that records after acting, or that abandons a frame and acts anyway, breaks the rule and loses records with nothing said. The channel cannot see either one.
4040

4141
## When the region fills up
4242

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
use std::{num::NonZeroU32, ptr::NonNull, sync::atomic::AtomicU64};
1111

1212
/// The CLOSED gate bit of the claim counter. The receiver sets it when it
13-
/// seals, and so does any failed claim, which is how it reports the loss
14-
/// (rule 1).
13+
/// seals, and so does a claim the region had no room for, which is how it
14+
/// reports the loss (rule 1). Nothing else sets it: what a writer cannot
15+
/// record for its own reasons is not this protocol's to report.
1516
///
1617
/// A bit rather than a value to compare against, so it survives the
1718
/// increment of a writer that arrives late. Counting cannot reach it: that

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

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
//! The channel asks one thing of its writers: **publish a record before
1010
//! performing the action it describes.** A record that never arrives then
1111
//! describes an action that never happened, and one refused after the seal
12-
//! describes an action performed after the receiver stopped collecting.
13-
//! [`ShmWriter::report_lost_record`] covers the case left over, where a
14-
//! writer gives up on a record and acts anyway.
12+
//! describes an action performed after the receiver stopped collecting. A
13+
//! writer that gives up on a record and performs the action anyway breaks
14+
//! the rule, and this protocol cannot tell that it did.
1515
//!
1616
//! `README.md` in this directory describes the region, the claim sequence,
1717
//! and why the receiver can borrow frames out of shared memory. [`layout`]
@@ -343,24 +343,6 @@ mod tests {
343343
assert!(sealed.unwrap_err() == SealError::Closed);
344344
}
345345

346-
/// A writer that gives up on a frame it already claimed, or before it
347-
/// could claim one, has no failed claim to report the loss for it.
348-
#[test]
349-
fn a_reported_loss_fails_the_seal() {
350-
let shm = MockedShm::alloc(1024);
351-
// SAFETY: see `single_thread_basic`.
352-
let writer: ShmWriter<_, S> = unsafe { ShmWriter::new(shm.clone()) }.unwrap();
353-
assert!(writer.try_write_frame(b"kept"));
354-
355-
assert!(!writer.is_closed());
356-
writer.report_lost_record();
357-
assert!(writer.is_closed());
358-
359-
// SAFETY: see `collect_frames`.
360-
let sealed = unsafe { ShmReader::seal::<S>(shm) };
361-
assert!(sealed.unwrap_err() == SealError::Closed);
362-
}
363-
364346
#[test]
365347
fn claims_after_seal_are_gated_without_poisoning() {
366348
let shm = MockedShm::alloc(1024);

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,6 @@ impl<M: AsRawSlice, const SLOTS: usize> ShmWriter<M, SLOTS> {
6767
Some(Self { mapped, _mem: mem })
6868
}
6969

70-
/// Reports a record this writer could not write, which closes the
71-
/// channel and fails its seal: whatever the receiver collects is no
72-
/// longer all of them.
73-
///
74-
/// A claim that fails for space reports itself. This covers a writer
75-
/// that gives up for its own reasons, before or after claiming, and
76-
/// then performs the operation the record described. Dropping the
77-
/// frame reports nothing on its own: the receiver cannot tell an
78-
/// abandoned slot from one whose writer died, and a writer that died
79-
/// never performed its operation.
80-
pub fn report_lost_record(&self) {
81-
self.mapped.claims().fetch_or(CLOSED, Ordering::Relaxed);
82-
}
83-
8470
/// Whether the CLOSED gate is set, by a seal or by a failed claim.
8571
pub fn is_closed(&self) -> bool {
8672
self.mapped.claims().load(Ordering::Relaxed) & CLOSED != 0
@@ -181,8 +167,9 @@ fn fitted_offset(start: u64, len: u32, payload_len: u32) -> Option<u32> {
181167
/// [`FrameMut::finish`] is the only way to show the payload to the
182168
/// receiver. Dropping the frame gives the claim up: the slot stays
183169
/// unfinished and the receiver ignores it, as if the writer had died
184-
/// there. A writer that drops a frame and performs the operation anyway
185-
/// owes the channel a [`ShmWriter::report_lost_record`].
170+
/// there. The two look identical from the outside, which is why a writer
171+
/// that drops a frame and performs the operation anyway breaks the rule
172+
/// this channel rests on.
186173
#[derive(Debug)]
187174
pub struct FrameMut<'a> {
188175
slot: &'a AtomicU64,

0 commit comments

Comments
 (0)