[Japabu]

Draft. Implements the owner's ruling on review r4's S1: one kernel call writes a message's bytes and moves its handles together, all or nothing, and the standalone handle send is retired. This supersedes the earlier readiness fix and the frame-first rule that was to accompany it; neither is in this branch.

Problem

SYS_HANDLE_SEND queued handles on a connection beside its bytes, and a connection's readiness was its bytes alone. A server waiting for bytes never saw handles sent with none and could not refuse them. Two connections sent over each other with no bytes kept each other open after their client died. Between two file-server directory connections, that leaks every description the client held and a share's served slot. Making queued handles readable instead (this PR's first version) would let any client spin every poll-driven server with one stray handle, because readiness is level-triggered.

Resulting behaviour

• SYS_MESSAGE_SEND (133) is the only way a handle crosses a connection. It takes bytes plus a handle list:

  • Checks: WRITE on the connection (and TRANSFER when it carries handles), TRANSFER on every handle, no duplicates and not the connection itself, all before anything moves. Handles need bytes (InvalidArgument).
  • Refusal: the message is refused whole when the peer holds MAX_QUEUED_BATCHES (ResourceExhausted) or is gone. It waits for room in the stream as a write does, or answers WouldBlock with MESSAGE_NONBLOCK. The work is one copy of at most MAX_MESSAGE_LEN = 16 KiB, charged to the caller.

• The batch is pinned at the stream position just past its bytes (kernel/pure/message.rs, with the pipe's new monotonic written/read counters):

  • a read never crosses a pinned batch, and answers HandlesPending (11) there;
  • SYS_HANDLE_RECV takes a batch only at the reader's position;
  • every sender and reader takes the receiver's queue lock, then the pipe lock.

  So a batch exists only with its frame, and a message's bytes wake the reader as any write does. Readiness changes in one place: a connection also reports readable while a batch waits at the reader's position, because a read there answers HandlesPending at once rather than blocking.

  • Unknown MessageSend::flags bits are InvalidArgument.

• SDK:

  • Header: the frame header is [msg_type][len][handles] (12 bytes), and every frame is one message_send.
  • Sending: send_with, signal_with, send_bytes_with and their try_ forms carry handles in their frame. TrySendError::Full now means nothing was sent. A payload of at most 64 bytes (every typed payload and signal) is framed in a buffer of that size, not an 8 KiB one.
  • Receiving: a reader takes exactly the announced batch with its frame, recv refuses a frame that carries handles, and FrameRx closes what its caller does not take (take_batch, take_handles::<N>). A batch no frame announced makes the read Malformed, and the server drops the connection.

• The other converted surfaces:

  • Senders and receivers: all 16 send sites, and the receivers in diskserver, compositor, fileserver, soundserver, hda, logkeeper, netstack, supervisor, blockserve, logkeeper-api and the window SDK.
  • Error mapping: std maps HandlesPending to InvalidData, and libc to EPROTO.
  • Removed: SYS_HANDLE_SEND, Connection::send_handles, recv_bounded, peek_width and the handles-before-the-frame rule.

Checks

Round 3 (head f8d512350, rebased onto d5713917d, #974): #974 took 131 and 132 for its DMA calls, so SYS_MESSAGE_SEND is 133 and SYSCALL_PROFILE_BINS is 135; SMALL_PAYLOAD's doc is reworded. The metal and QEMU evidence at b705b266c carries over: the rebase changes only that number and brings in #974's DMA code, which none of this branch's paths touch. Rerun at f8d512350 as the smoke check:

check	exit
cargo run -- --ci host	0 (88 steps)
cargo run -- --build-only (x86_64), --arch aarch64	0, 0
QEMU x86_64 console_image_boots (every boot's file-server HELLO carries its window in its frame)	0 (1/1)

Round 2 (review r1's fixes, head b705b266c; base e62ebc084):

check	revision	exit
cargo run -- --ci host	da07292cc + the fixes, i.e. b705b266c before its issue-text edit	0 (87 steps)
cargo run -- --build-only (x86_64), --arch aarch64	same	0, 0
cargo test -p kernel --lib --features sched-check message::	b705b266c	0 (4 tests)
negative control: Batches::window ignores pins (patch applied, built, restored)	b705b266c	101: both stream tests red
negative control: admit takes handles without bytes	b705b266c	101: a_refused_send_leaves_nothing_behind red
QEMU x86_64, the 12 rows below	b705b266c	0 (12/12)

The QEMU rows are console_image_boots nvme_disk_boots desktop_without_x2apic the_compositor_drives_the_virtio_gpu a_window_is_drawn_at_the_scale_and_relaid_at_the_next hda_plays_to_the_controller_that_reaches_an_output libc_children a_launch_gives_each_unsent_stdio_slot_a_null machine_shutdown_wire_held machine_shutdown_wire_kept a_drives_row_holds_one_process_per_function the_opening_exchange_with_qemus_agent_completes. They cover the converted protocols QEMU can reach: the fileserver HELLO, disk and blockserve, compositor and window, soundserver and hda, launcher, logkeeper and shutdown, and the agent exchange. Netstack has no QEMU row; its frames are covered by host tests and by the metal shared boot.

Round 1 (96d307a12): the same checks, all 0, plus the T14 results posted on this PR: fix 11/11 across 3 boots; the FrameRx::release control red as predicted. Round 2 changes the send path, admission and the write-room check those images exercise, so they are re-staged below rather than reused.

Evidence (masked): round 3 https://github.com/ToyOSOrg/evidence/tree/ac745feb80c2acddb48ae074b6b078220f6c01f6/r3 and round 2's T14 readbacks https://github.com/ToyOSOrg/evidence/tree/ac745feb80c2acddb48ae074b6b078220f6c01f6/r2/metal-run; round 2 https://github.com/ToyOSOrg/evidence/tree/34e2c0c2a9e376d6952737f116e02eb104766d31/r2, round 1 and its T14 readbacks https://github.com/ToyOSOrg/evidence/tree/34e2c0c2a9e376d6952737f116e02eb104766d31/r1

Metal at b705b266c (T14; carries over to round 3)

The new guest binaries are discovered Rust tests, and those run only in the T14's shared boots; QEMU has no shared Rust block.

• Fix, 3 images: message_carries_its_handles, ends_sent_over_each_other_both_end, and the converted process_tree spawn_lands_claimed cpal_drop_unreleased abuse_shared_grant mutual_kill shm_release_reclaims pipe_other_end_gone stream_handles netstack_gone_mid_bind.
  Results (readbacks in evidence r2/metal-run/): fix 11/11 passed across 3 boots; the kernel control failed only message_carries_its_handles, at "the reader went past a batch no frame announced", with ends_sent_over_each_other_both_end green, as predicted.

• Kernel control, 1 image: HandleQueue::read passes a window that ignores pins. An SDK reader of an announced frame reads exactly its header and then its payload, so it lands on the pin anyway, and the image boots and serves as usual. Expected: message_carries_its_handles red at "the reader went past a batch no frame announced" (part 3, where the second pump answers Idle), and ends_sent_over_each_other_both_end green.

Decisions and risks

• A plain write on a connection still exists. It carries bytes only: doorbells use it, and it cannot attach a handle. A pin uses the end position its own whole write returned, so an interleaved plain write cannot misplace a batch.
• No loom test (review r1 agrees). The atomicity is mutual exclusion: one queue lock held from admit to pin, and taken by every reader. The host model tests the ordering logic; the kernel's composition of the read is covered by the kernel control above.
• A receiver whose own handle table is full drops the peer as Malformed. The batch stays pinned, so the reader cannot pass it; dropping the connection is the only bounded answer, whichever side caused it (review suggestion S2, declined).
• The ABI changes: syscall 103 is gone, 133 is new, the frame header grows from 8 to 12 bytes, and SyscallError::HandlesPending is new. The ABI is unstable, and every consumer in the tree is converted.

🤖 Generated with Claude Code

[Japabu]

Metal, T14 (ThinkPad T14 Gen 2), head 96d307a12, run by the orchestrator, all four boots in one session:

• Fix: the proctreecase, shared-debug and testcases images all booted (rc 0). The judge reported 11 passed, 0 failed: message_carries_its_handles, ends_sent_over_each_other_both_end and the 9 converted binaries.
• Control (FrameRx::release closes nothing): message_carries_its_handles PASS. ends_sent_over_each_other_both_end FAIL (101) with "two ends sent over each other kept each other open", as predicted.

Logs stay in the readback directories; the implementer publishes them to evidence pr-973.

[Japabu]

Review r1: PR #973 at 96d307a12 (base e62ebc084)

Architecture

The branch implements the S1 ruling. One syscall (SYS_MESSAGE_SEND, 131) writes a frame's bytes whole and pins the batch at the stream position just past them. The standalone send (103), send_handles, recv_bounded, peek_width and the handles-before-frame rule are all deleted. The ownership is right. The kernel owns atomicity and placement (kernel/pure/message.rs plus HandleQueue). The SDK owns the frame's announced count. Each server owns its protocol response, which is to drop the connection. Pinning onto the existing byte stream is a small change, and it keeps plain writes available for doorbells. I see no simpler representation that keeps stdio and doorbell byte streams working.

Size: +1306/−477. About 330 of the added lines are tests: the message.rs tests, two guest binaries and the ipc.rs tests. Net production growth is roughly +500, mostly the SDK send/receive surface and FrameRx batch ownership.

Atomicity (checked by tracing the code; I ran nothing). sys_message_send takes the sender's process-data lock. It validates WRITE, plus TRANSFER when handles are present. It then checks TRANSFER on every handle, duplicates and the connection itself, all before transfer takes anything. HandleQueue::send holds the peer inbox lock across admit, pipe::write_whole (all-or-nothing under PIPES) and pin(end). Every refusal (Empty, QueueFull, NoRoom, BrokenPipe, NoMemory, inbox None) returns the batch, and transfer gives each entry back at its own number. No handle is dropped under a lock. Cancellation and WouldBlock leave nothing moved. User buffers are pinned before any lock is taken (user_ptr pins frames for the copy's life), so no copy faults under PIPES.

Read side. read, take and pinned_here all take the inbox lock and then PIPES, in the same order as the sender. I found no path that takes PIPES and then a queue lock. try_read_within limits a read to the pin and answers HandlesPending at it. SYS_HANDLE_RECV takes a batch only at the read position, and the width and table-room checks run before the take. Plain writes do not take the queue lock, but they cannot misplace a batch. pin uses the end returned by its own write_whole, and a reader cannot run between the write and the pin because it needs the queue lock held across both. No other consumer advances the ring: SYS_PIPE_MAP exposes data only, and the cursors stay in kernel memory.

Loom. I agree that no loom test is needed. The positions, pins and ring cursors all change only under the queue lock and PIPES. Nothing here is lock-free. The only two-step check is ConnectionEnd::has_data (pipe check, then pin check). It decides readiness only, and a send posts the read watch after it pins. The host model plus inspection is the right evidence.

Send sites. I counted all 16 production sites (fs HELLO, launch, net, compositor ×4, diskserver ×2, fileserver, logkeeper, netstack, soundserver ×2, supervisor ×2) and the 8 guest-test sites. All are converted. Each receiver either takes exactly the announced batch or lets FrameRx or Batch close it. For each close path I checked, no handle leaks and none is closed twice: recv_handles, recv_handles_exact, recv, FrameRx::pump/Drop, Batch::exact/Drop/into_raw, fs call/stream, and netstack lend. Raw doorbell readers (blockserve, diskserver session, netstack drained/gone) treat any error other than WouldBlock as a hang-up. So an unannounced batch on a doorbell ends the session; it does not spin.

Metal. The T14 results posted on the PR match the plan. All 11 fix binaries pass in the 3 images. The SDK control (FrameRx::release closes nothing) turns ends_sent_over_each_other_both_end red and leaves message_carries_its_handles green, as predicted. That control is adequate for the SDK claim. It is not adequate for the kernel claim; see B1.

Blockers

B1 — PR description, "Metal: staged" section — false evidence claim, and no control covers part 3 of message_carries_its_handles. The PR says: "A kernel read that ignores pins would also break every server's HELLO, so its image would fail at boot without isolating anything." That is false. FrameRx::fill, read_exact and recv_payload always read exactly the header (12 bytes) and then exactly len bytes. A reader of a properly announced frame therefore lands exactly on the pin, and take(read) succeeds whether or not window honours pins. A kernel whose try_read_within ignores pins would still boot and serve every HELLO. The only check that would go red is part 3 of message_carries_its_handles: the second pump would answer Idle instead of Malformed.

The host controls cover Batches::window. They do not cover the composition in HandleQueue::read (kernel/src/object/service.rs:67), which is what part 3 exists to establish. Required: delete the false sentence. Then either add the kernel-side control image to the next metal batch (try_read_within called with a window that always answers Bytes(available); expected: message_carries_its_handles red at "went past a batch no frame announced"), or state explicitly that part 3 rests on inspection of those five lines.

B2 — tests/guest/rust-tests/src/bin/message_carries_its_handles.rs:85-105 — inline asm! in a test binary, testing what inspection establishes. Assembly belongs in the crate's arch module (tests/guest/rust-tests/src/arch/). Older binaries breaking that rule does not license another. The probe only shows that number 103 now reaches the dispatcher's unknown-number arm, and that is visible in dispatch.rs. Delete the retired_handle_send probe and its assertion. Keep the message_send(&[], [h]) refusal check, which tests real kernel code.

B3 — kernel/src/syscall/dispatch.rs:453-468 — unknown MessageSend::flags bits are silently ignored. Other flag-taking syscalls refuse unknown bits at the boundary: ipc.rs:97 (NAMESPACE_FLAGS_KNOWN), vm.rs:39, proc.rs:70. Refuse args.flags & !MESSAGE_NONBLOCK != 0 with InvalidArgument. Otherwise this ABI accepts garbage and cannot add a flag later without changing the meaning of existing calls.

B4 — kernel/pure/message.rs:65 and kernel/src/object/service.rs:44-52 — production code that exists only for the model test. The kernel calls admit(len, carries, cap, usize::MAX, true). Its space and reader parameters, Refused::NoRoom and Refused::Gone, and the two match arms that map them can never fire in the kernel. Room and reader are answered by write_whole, as the comment at service.rs:45 admits. Those parameters are exercised only by a_refused_send_leaves_nothing_behind. Narrow admit to (len, carries, cap) with Empty | QueueFull. Have the model's Stream::send check its own room. Drop the dead arms and the compensating comment. Also delete the unused Batch::len/is_empty (sdk/toyos/src/ipc.rs:712-718; nothing calls them) and merge the two impl Batch blocks.

B5 — stale or false comments left by the change.

• system/compositor/src/client.rs:296-299: "TrySendError::Full can have left part of the frame in the pipe, so the peer's stream is past saving". False now: Full means nothing was sent. Shorten it to say the failure is not retried and the window is dropped.
• system/netstack/src/client.rs:59-63: the same false claim, plus "ipc::send parks in sys_write", which is now SYS_MESSAGE_SEND.
• sdk/toyos/src/ipc.rs:156-163: MAX_TYPED_PAYLOAD is now dead. Nothing references it, and its doc describes the try_send buffer and compile-time assert this PR deleted. Delete the constant and its doc.
• sdk/window/src/lib.rs:89, :252, :294: "sent ahead of the frame". The handle now travels in the frame.
• sdk/toyos/src/audio.rs:226-227: "Handles cross before the frame that names them". Same.

B6 — issues/a-moved-handle-is-always-re-movable.md:53-54 — a quotation was rewritten. The text in quotation marks is attributed to "the shared-memory grant's old argument", and the edit replaced SYS_HANDLE_SEND inside it. Restore the quoted wording. Update only the sentence after it, which states the current fact.

Suggestions (optional)

• S1 — sdk/toyos/src/ipc.rs:420. Every send, including bare signals and 12-byte mouse events, now zero-fills an 8 KiB stack array. The deleted try_send_bytes doc had kept that buffer off per-frame paths for this reason. Options: give typed payloads a header-plus-MAX_TYPED_PAYLOAD buffer as before; use MaybeUninit; or let the ABI take the header and payload as two segments, so no assembly copy is needed.
• S2 — sdk/toyos/src/ipc.rs:683. handle_recv(..).unwrap_or(0) discards the reason. A receiver whose own table is full (ResourceExhausted) then drops a well-behaved peer as Malformed, which blames the peer for a local shortage. Consider passing the error through, or at least telling the two apart in the step.
• S3 — kernel/src/pipe.rs:303. has_room(id, len) nearly duplicates has_space. It could be has_space with a length, so writability has one definition.

For the orchestrator (PR text)

• "readiness needs no change" is inaccurate. ConnectionEnd::has_data now also reports a batch pinned at the read position (service.rs:184). That change is correct, because a read there does not block (it answers HandlesPending), so level-triggered readiness must say readable. But it is a readiness change, and the description should say so. It does not conflict with the ruling, which said no change was needed, not that none was allowed.
• The B1 sentence needs removing.

SEND BACK

[Japabu]

Review r1, round 2, at b705b266c (on top of da07292cc, the two issue records):

• B1: the false sentence is gone. A kernel-side control image (HandleQueue::read passes a window that ignores pins) is staged for the T14. Expected: message_carries_its_handles red at part 3, and ends_sent_over_each_other_both_end green.
• B2: the inline-asm probe of 103 is deleted; the message_send(&[], [h]) refusal stays.
• B3: SYS_MESSAGE_SEND refuses flag bits other than MESSAGE_NONBLOCK with InvalidArgument, and the field's doc says so.
• B4: admit(len, carries, cap) with Empty | QueueFull only. The kernel's dead arms and comment are gone; the model's Stream::send checks its own room. Batch::len/is_empty are deleted and the two impls merged.
• B5:
  • The comments at compositor client.rs, netstack client.rs, the three sdk/window sites and audio.rs are corrected.
  • MAX_TYPED_PAYLOAD is replaced by a private SMALL_PAYLOAD, which S1 uses.
• B6: the quotation is restored; only the sentence after it states the current fact.
• S1, taken: payloads of at most 64 bytes, every typed payload and signal, are framed in a 76-byte buffer; larger ones in the 8 KiB one.
• S2, declined: with the receiver's own table full, the batch stays pinned, so the reader cannot pass it. Dropping the connection is the only bounded answer, whichever side caused it. This is now stated in the PR body.
• S3, taken: Holders::writable(space, want) is the one definition. A byte write's readiness is has_space(id, 1) and a message's wait is has_space(id, len); has_room is gone.
• PR text: the readiness change (a batch at the reader's position is readable) is now stated.

Checks:

• --ci host 0 (87 steps), both builds 0 and the 12 QEMU rows 12/12.
• The host controls are regenerated against the narrowed admit: both red (101), base 0.
• Round 1's 11/11 does not carry over, because round 2 changes the send path, admission and the room check those images exercise. The fix images are re-staged with the control.

Evidence: https://github.com/ToyOSOrg/evidence/tree/34e2c0c2a9e376d6952737f116e02eb104766d31/r2

[Japabu]

r2: b705b266c (on da07292cc)

I checked only the fixes and the code they changed (git diff 96d307a12 b705b266c), plus the PR body.

r1 blockers

• B1, resolved. The false sentence is gone. The new staging describes the right kernel control: HandleQueue::read passes a window that ignores pins. The predictions are right. message_carries_its_handles should go red at part 3, because the second pump answers Idle. ends_sent_over_each_other_both_end should stay green, because announced frames land on their pins. The r1 FrameRx::release control still stands, because r2 does not touch FrameRx release. The orchestrator's T14 run will settle the control.
• B2, resolved. The asm probe and the 103 constant are deleted. The message_send(&[], [h]) refusal check stays.
• B3, resolved. dispatch.rs:453 refuses flags & !MESSAGE_NONBLOCK with InvalidArgument before any user buffer is pinned. The ABI field doc states the rule.
• B4, resolved. admit(len, carries, cap) now returns only Empty | QueueFull. The kernel's two dead arms and the compensating comment are gone. The model Stream::send checks room itself and returns None for no room, so the model tests no longer need production code. The admit control was regenerated against the narrowed admit: 101, base 0. Batch::len/is_empty are deleted and the two impl blocks merged.
• B5, resolved. The comments at compositor client.rs:296, netstack client.rs:59, the three sdk/window sites and audio.rs:226 now state the current behaviour. MAX_TYPED_PAYLOAD is gone.
• B6, resolved. The quotation reads SYS_HANDLE_SEND again. Only the sentence after it states the current fact.

Taken suggestions

• S3, has_space(id, want): correct. Holders::writable(space, want) is space >= want.max(1) || !held(Read), so has_space(id, 1) keeps the old readiness and blocking-write meaning (space > 0). The message wait uses has_space(tx, len). Zero-length messages never reach it, because write_whole accepts zero bytes. len <= MAX_MESSAGE_LEN fits in a u32, so the saturating conversion cannot bite. has_room is gone, and every caller is converted (ops.rs, io.rs, ipc.rs).
• S1, SMALL_PAYLOAD: correct. The closure frames into a 76-byte array for payloads of 64 bytes or less, otherwise into the 8 KiB one. The slicing is bounded by the header check that runs first, so the small path no longer zero-fills 8 KiB. One optional nit: the doc says "every typed payload and signal", but nothing enforces it any more, because the old compile-time assert went with try_send's buffer. Today it is true: the largest payloads I found are fs Reply at 48 bytes and Request at 40. Either add the assert to the typed send* paths or write "small payloads". This is not blocking, because a larger payload is still sent correctly through the large buffer.

S2 decline

I accept it. With the receiver's table full, take refuses and leaves the batch pinned, so read keeps answering HandlesPending. The connection cannot make progress whoever is at fault, so dropping it is the only bounded outcome. My point was only that the step names the cause Malformed. That is a diagnostic detail, not a behaviour defect, and the PR body now states the trade-off.

Other changes in the branch

da07292cc adds issues/a-pipe-write-of-a-few-bytes-can-tear-between-two-writers.md (status owner, question) and a paragraph in four-shares-take-a-file-server.md. They record unrelated discoveries with an owner and an exit, and change no code. The new PR text is accurate: the readiness change is described, the false HELLO claim is gone, and the evidence revisions are named.

No blockers remain. The verdict below rests on review alone. The re-staged metal run (3 fix images plus the kernel control) is still the orchestrator's to read, and the control must come out red at part 3 as predicted.

LAND

[Japabu]

Metal, T14 (ThinkPad T14 Gen 2), head b705b266c, run by the orchestrator, one session:

• Fix: the proctreecase, shared-debug and testcases images all booted (rc 0). The judge reported 11 passed, 0 failed.
• Kernel control (HandleQueue::read ignores pins): ends_sent_over_each_other_both_end PASS. message_carries_its_handles FAIL (101) with "the reader went past a batch no frame announced", as predicted.