Skip to content

netstack: a listener and a datagram socket are their holders; no request names a socket by a number - #1052

Merged
Japabu merged 4 commits into
mainfrom
wt/netstack-socket-holders
Oct 23, 2026
Merged

Japabu merged 4 commits into
mainfrom
wt/netstack-socket-holders

Conversation

@Japabu

@Japabu Japabu commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

netstack named listeners and datagram sockets by u32 ids. Any process holding the netstack connector could close, accept on or receive on another process's socket by naming its number (issues/netstack-socket-ids-are-ambient.md; a service swap once closed the wrong socket and crashed netd).

Resulting behaviour

A listener and a datagram socket are now what a stream already is: objects named only by what their holders hold.

  • Holders. A listen or a bind is a request on a new connection. That connection becomes the socket's first holder and is answered with a bell netstack made. Every later request is a frame on a holder's own connection and acts on its own socket: ACCEPT (with the stream's region; answered with the stream's connection and bells), SEND / RECEIVE (one datagram per frame each way), SET_OPTION, OPTION, EXPORT. No frame carries a socket id. A holder frame on a new connection is no request there (ERR_INVALID_INPUT); a frame a holder of that kind does not send drops the holder.
  • A look asks only while its bell is rung. An accept or receive checks its bell's readiness without reading it; an empty bell waits, or answers WouldBlock, with no request and no region. A rung look makes its slot and region and reads the bell empty just before the request goes, so a look that fails first leaves the ring where pollers see it. An export rings its new holder at once of what waits.
  • One queue, no lost wake. The node keeps each socket's holders (system/netstack/node/src/holders.rs). A holder is rung while a connection or datagram waits and it has not asked since it was last rung. All holders take from one queue; the loser of a race gets ERR_WOULD_BLOCK (NetError::WouldBlock). The client reads its bell only just before a request netstack then answers. A holder past the first takes a place; the socket ends with its last holder.
  • Handover. export makes another holder through netstack's private port; its first frame is socket::ADOPT. An exported end now says what it holds, so one descriptor kind, fd:<n>:exported, covers streams and sockets (net::adopt_exported). std's spawn and fd::copy export listeners and datagram sockets as they did streams, so a later libc fork can use the same path.
  • Clients in one landing. toyos::net::socket keeps the process's ends in the table stream.rs now shares (net/ends.rs). std's TcpListener / UdpSocket and libc's sockets sit on it. UDP gains real nonblocking and read timeouts; broadcast() / getsockopt read netstack's value; std gains FromRawFd/From<OwnedFd> for both types. A libc socket that binds becomes netstack's object, so posix_spawn hands it on. mio's code needs no change, since a socket descriptor's handle is its bell; its listener comments are corrected in fork commit 5d013b8, which the lock names.

Deleted: the id table and its random start, the client-made notify/data pipes, the per-connection wake count, wake-pipe refusals ending a listener, and netstack's waiting UDP receive. The branch adds about 730 lines overall, 540 of them outside the test directories.

Decisions worth review

  1. A listener holder's connection keeps TRANSFER. The kernel carries handles only on a connection with TRANSFER, and an accept carries its region. The stream rule (holder handles carry no TRANSFER) still holds for the bell, which is a socket descriptor's handle, and for a datagram socket's connection. No descriptor names the listener's connection handle. The first guest run went red on exactly this: the child's accept failed PermissionDenied.
  2. Datagrams are frames on the holder connection, as the brief directs. netstack-moves-every-byte-through-two-kernel-pipes' stage 4 planned a region per datagram socket. I rewrote stage 4 to say what is left: the region, since a datagram is still copied through the connection's pipes.
  3. A datagram socket's peer is netstack's (owner's ruling, review r1). A holder's CONNECT connects [udp]'s socket, so the peer and its receive filter are the socket's and every holder's; PEER reads it and SEND_PEER sends to it. A connected socket's pending ICMP error rings its holders and is answered once. libc's connect of an unbound datagram socket binds it, as a first send does.
  4. Issues. Closed: netstack-socket-ids-are-ambient; an-accept-that-never-reaches-netstack-strands-its-listener, whose mechanism (a spent wake netstack still counted) is gone; and netstack-holds-a-pending-request-for-a-client-that-left, since no receive waits in netstack. only-a-connected-stream-is-shared-with-a-child is renamed a-socket-libc-or-socket2-has-not-bound-is-not-shared-with-a-child, with what remains. Other records naming the old protocol are updated.

Checks (revision f12211e, macOS host)

check result
cargo run -- --ci host exit 0, 103 steps green
cargo run -- --build-only exit 0
cargo test --test toyos-build -- netstack_holders_exported_to_children stream_holders libc_children iommu_virtio_platform 3 pass, holders row red (first run)
cargo test --test toyos-build -- netstack_holders_exported_to_children after the TRANSFER fix pass
mutation: a holder's ask does not reset its ring 11 node tests red (a_holder_that_never_looks…, every_holder_takes_from_one_queue…, datagram queue test…), restored
mutation: a datagram holder may ACCEPT a_holder_asks_only_its_own_kind_of_socket red, restored

Logs: https://github.com/ToyOSOrg/evidence/tree/c6ba67bde5f02cb735d9150995095804562df7e5/f12211e8f (ci-host-1, build-2, guest-2 the red run, guest-3 the green one, m1, m2, metal-request.txt).

The first holders-row run is also its negative control: a child whose accept failed turned the row red.

New host tests: node holder laws (one queue across holders, a never-looking holder costs the other nothing, last-holder close, places), datagram socket holders (each datagram once and whole), the serve boundary (a_new_connection_names_no_socket, a_holder_asks_only_its_own_kind_of_socket), and datagram framing round trips.

New guest job socket_export, in the renamed row netstack_holders_exported_to_children. It shares one netcase boot with stream_export. A child accepts on a listener and exchanges datagrams over a socket handed to it; then the parent does the same through its own holders. The row needs a guest because export, endowment, adoption and real netstack cross real processes. The harness is the peer through a new BootOptions::forward (a QEMU user-net hostfwd of one loopback port; refused on a profile with no NIC). It runs on QEMU x86-64 only (SUITE_ARCH) and touches no public network.

The swap case is not rebuilt: swap_crash_rolls_back and netd are gone, and no request can carry a stale id any more.

Metal

None of the claims need metal: they are about order, contents and counts, with no timing or device behaviour. netstack_gone_mid_bind, which I edited (its two arms now use a connect's and a listen's shapes), runs only in the T14's shared boot. It is staged with --metal-readback.

Review r1 (head 2222561)

All three blockers fixed as above. Checks at 2222561: cargo run -- --ci host exit 0, 103 steps; cargo run -- --build-only exit 0; netstack_holders_exported_to_children, stream_holders, libc_children pass. socket_export now also races the parent and a second child, both blocked in accept and then recv: the harness connects twice and sends two datagrams, each process takes one of each, and the harness checks the job's claim lines. New host tests: connected peer shared by holders, a new holder rung at once (mutation dropping that ring: a_holder_made_while_a_connection_waits_is_rung_at_once red, restored). Logs: https://github.com/ToyOSOrg/evidence/tree/f0d7342f5e2ef6b6f36192d22f3e8206277678ef/22225610b. Metal request restaged at this head.

Review r2 (head 032114b)

Both blockers fixed as above; a datagram receive also rings the holders whatever it answers, so a pending ICMP error no longer strands the datagram behind it. a_pending_error_is_answered_once_and_the_datagram_behind_it_rings_again was red before that fix (r2-before-fix.log: the asker's ring count stayed 1). Checks: cargo run -- --ci host 102 of 103 green, the red one clippy's unused sum import in node/tests/resolve.rs, fixed and rechecked with cargo clippy -p toyos-net-node --all-targets -- -D warnings (exit 0); netstack_holders_exported_to_children, libc_children, iommu_virtio_platform pass; stream_holders passed on its own rerun after the combined run's worker sat 20 minutes on a sysroot lock its own process held, unrelated to this change. Logs: https://github.com/ToyOSOrg/evidence/tree/7ed116016badcda0e4821ae4e5f99921fb0feb22/032114b1d. Not restaged for metal: neither fix touches netstack_gone_mid_bind's path.

Review r3 (head e5f90fd)

Each socket end makes its own poller when the end is made, and keeps it in the end's turn. A look that does not wait asks the kernel for nothing it can refuse, and making an end answers ResourceExhausted if the kernel makes no poller. A blocking wait runs outside the turn, which the end's other calls take meanwhile, so it makes a poller of its own with the new Poller::try_new and answers ResourceExhausted where it cannot. The module's no-lost-ring sentence now covers only a call that fails before its request goes. The round 2 sysroot-lock wedge is recorded in the existing issues/a-suite-that-builds-its-sysroot-can-wedge-on-its-own-locks.md, not as a new issue.

Checks at e5f90fd: cargo run -- --ci host exit 0, all 103 steps green; netstack_holders_exported_to_children and iommu_virtio_platform pass. Logs: https://github.com/ToyOSOrg/evidence/tree/611bab2b37de1355b663add6094bcde434ff19d0/e5f90fd5f.

Risks

  • A survivor after its other holder is killed is not tested in a guest (left in the renamed issue).
  • MAX_DATAGRAM (1472) is declared in toyos::net::socket beside udp's MAX_PAYLOAD, with a compile-time check that the second fits the first.

🤖 Generated with Claude Code

…request names a socket by a number

netstack named listeners and datagram sockets by u32 ids that any client holding the
`netstack` connector could close, accept on or receive on
(issues/netstack-socket-ids-are-ambient.md). Each is now what a stream already is: an
object named only by what its holders hold.

- A listen or a bind is a request on a new connection, which becomes the socket's first
  holder and is answered with a bell netstack made. Every later request is a frame on a
  holder's own connection, acting on its own socket: ACCEPT (with the stream's region,
  answered with the stream's connection and bells), SEND and RECEIVE (one datagram per
  frame each way), SET_OPTION, OPTION and EXPORT. No frame carries a socket id; a holder
  frame on a new connection is no request there, and a frame a holder's kind does not
  send is its leaving.
- The node keeps each socket's holders (holders.rs): every holder is rung while a
  connection or a datagram waits and it has not asked since it was last rung, so all
  holders take from one queue, the loser of a race is answered ERR_WOULD_BLOCK, and no
  ring is lost. A holder past the first takes a place; the socket ends with its last
  holder. The per-connection wake count, the wake-pipe refusals that ended a listener,
  the client-made notify and data pipes and the waiting receive are gone.
- An export makes another holder through netstack's private port, its first frame
  socket::ADOPT. An exported end now says what it holds, so a descriptor's kind is one
  `fd:<n>:exported` for streams and sockets alike (net::adopt_exported); std's spawn and
  fd::copy export listeners and datagram sockets as they did streams.
- toyos::net::socket keeps the process's ends in the table stream.rs now shares
  (net/ends.rs). A descriptor's handle is a copy of the bell, which pollers watch (mio
  unchanged) and no file call reaches. A listener holder's connection keeps TRANSFER: the
  kernel carries an accept's region only on a connection with it, and no descriptor
  names that handle.
- std's TcpListener and UdpSocket and libc's sockets sit on it: UDP gets real
  nonblocking and read timeouts, broadcast reads netstack's value, and libc's bound
  socket is netstack's object, so posix_spawn hands it on.

Issues: netstack-socket-ids-are-ambient, an-accept-that-never-reaches-netstack-strands-
its-listener (a ring is read only just before a request netstack then answers) and
netstack-holds-a-pending-request-for-a-client-that-left (no receive waits in netstack)
are closed; only-a-connected-stream-is-shared-with-a-child is renamed to what is left.

Tests: node holder laws (one queue, no lost ring, last holder, places), the holder
decoder's boundary, datagram framing; netstack_holders_exported_to_children runs the
stream export beside a new job in which a child accepts on and exchanges datagrams over
sockets handed to it, then its parent, through a forwarded QEMU port.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1: PR #1052 at f12211e

Architecture

The security fix holds. No request names a listener or a datagram socket any more. A socket is now the set of connections that hold it, and each frame acts only on its own holder's socket. git grep at f12211e finds no socket-id request path left. The only socket_id is a local in serve.rs:580, and it holds a watch token's key. The new connection's MsgType set (8/12/23/25) no longer overlaps any holder frame number; a_new_connection_names_no_socket checks this. Ownership is right:

  • the node owns the queues and the ring law (holders.rs);
  • serve.rs routes frames by connection;
  • toyos::net::socket owns the process's end;
  • ends.rs is a real shared table with two users (stream and socket), and it drops the multi-handle generality that nothing on main ever used.

The protocol lives in toyos::net, not toyos-abi. std, libc and the supervisor change in the same landing. The mio and socket2 pins name no removed API (checked at 5c122cf and 2aa6b00).

Boundary checks I traced:

  • Wrong-kind frames. asked() admits a frame only in its exact kind, shape and handle count. Anything else drops the holder. Stream frames on a socket holder, socket frames on a stream holder (heard) and holder frames on a new connection are all refused, and none of these paths panics. m2 (a datagram holder allowed to ACCEPT) turns a_holder_asks_only_its_own_kind_of_socket red.
  • Accept. A bogus region is judged only after the node has found a connection and a place (take: tcp_ready, then session()), so it consumes no connection. The holder is rung again (an_accept_whatever_it_answers_is_a_look).
  • Datagrams. Each datagram is one frame each way. A process's threads take turns through TURNS, and different holders have different connections, so nothing can interleave. The receiver checks n == header.len() against MAX_FRAME.
  • Export. The exported end is a private-port connection that netstack registers as one more holder. It is not a connector, and any request frame on it drops it. It grants only that socket. The adopted bell and a datagram holder's connection are narrowed to no TRANSFER.
  • The single queue. The node's law is "rung while something waits and not told since the last ask". I find it sound, and m1 turns 11 tests red. On the client side, the bell is read only inside an attempt that then asks. A failure before the ask (slot, region, connect) happens before the drain, so it reads no ring. A loser is answered ERR_WOULD_BLOCK, which becomes WouldBlock/EAGAIN, or it waits on the bell, which the node rings again because the loser just asked.

The three flagged decisions:

  1. The listener holder's connection keeps TRANSFER. Accepted. The kernel requires TRANSFER on the connection itself to carry handles (kernel/src/syscall/ipc.rs:366). The alternatives are worse: a region netstack makes defeats charging the client for it, and a two-phase accept adds a state. The process can only move its own holder. The module doc states the exception.
  2. Datagrams as frames; stage 4 rewritten. Accepted. The region plan stays as stage 4's remaining work, so no planned capability is dropped. One stale line is noted under suggestions.
  3. The UDP default peer is per process. Not accepted as it stands. See blocker 2.

The closed records are correct. netstack-socket-ids-are-ambient's exit is met. The mechanisms of an-accept-that-never-reaches… and netstack-holds-a-pending-request… are gone; the accept's failure steps now come before the drain, and no receive waits in netstack. No citation of the three slugs or the old slug survives outside issues/. The rename moved every citation.

Size. Overall +732 lines (2605/1873). Outside the test directories it is +540, including in-file unit tests. Test directories add +281 and issue records −89.

Findings

BLOCKER 1: sdk/toyos/src/net/socket.rs:191 (waited), :327 (accept), with sdk/toyos/src/net/stream.rs:750,817. Every accept attempt makes an 8 MiB stream region before netstack can say nothing waits.

  • What happens. waited always attempts before it looks at the bell. stream::accept runs inside open, which first calls SharedMemory::create(REGION_BYTES) and touches the header, so at least one 2 MiB page is faulted and zeroed. It then lends the region. On ERR_WOULD_BLOCK, both sides drop it.
  • Where it costs. An idle blocking accept (std's incoming() loop) makes and destroys a region before it waits. Each mio/tokio readiness round ends in a nonblocking accept that does the same. So does every loser and every stale ring.
  • Regression against main. Main's std read the wake byte first and made a region only when a connection was announced.
  • Under strict commit (mm: a commitment is promised its frames when it is made (memory stage 3b, strict commit) #1010). An accept with nothing waiting can now fail ResourceExhausted instead of blocking or answering WouldBlock.
  • The PR's own law allows avoiding it. Every drain is followed by an ask. After any ask the node rings again if something is left, and it does so before the answer is sent: Node::accept and udp_recv_from pass or tell before serve replies. So an empty bell means "wait" unless this end read a ring and has not asked since.
  • Suggested fix.
    • Attempt only when the bell rang, or when the end holds a ring it read and has not yet followed with an ask. The second case is one flag per end, set when a drain reads a ring and cleared when an ask is answered, so a failure after the drain keeps its ring.
    • Otherwise wait, or answer WouldBlock with no IPC.
    • Node::export_listener and export_datagram must then tell the new holder at once, because nothing guarantees another pass before its first call.
    • The same gate removes the round trip from every nonblocking recv_from that finds nothing.

BLOCKER 2: sdk/toyos/src/net/socket.rs:153,379, sdk/libc/src/socket.rs (connect on a datagram socket). The default destination is a second, process-local copy of state that [udp] already owns. The gap it leaves is recorded nowhere.

  • toyos_net_udp already implements connected sockets: udp.connect-*, ConnectedDestinationMismatch, RxDroppedOnConnect and ICMP errors to connected sockets (system/netstack/udp/src/lib.rs:56-88).
  • This PR adds a new End::peer in the SDK instead. It is not carried to an exported holder. A child given a connected UDP socket gets ENOTCONN from send(), and receive filtering by peer is missing as well.
  • That contradicts the owner's ruling in the renamed issue that inheritance is preserved for every socket kind. The gap appears only in the PR body and a doc comment.
  • Preferred fix: a holder CONNECT frame onto [udp]'s connect. The peer then becomes socket state shared by every holder, and End::peer and its std copy go away.
  • Minimum fix: record the gap with owner and exit in issues/a-socket-libc-or-socket2-has-not-bound-is-not-shared-with-a-child.md, and treat it as the owner's call.

BLOCKER 3: mio fork src/net/tcp/toyos_listener.rs:10-11,24 at the consumed pin 5c122cf. Its comments now describe the deleted protocol.

  • It says the handle is "the pipe netstack writes one wake into for each connection that waits" and that accept is "one wake taken, then std's accept".
  • Under ring-once-per-look a consumer must accept until WouldBlock, so a maintainer who trusts the comment writes exactly the wrong loop.
  • "mio needs no change" is true of the code, not of the comments. Fix them in a fork commit and bump the pin in this landing (implementer.md's fork rules).

BLOCKER 4 (PR description; the orchestrator's to correct): "Net −150 lines" is false. The branch is +732 overall and +540 outside the test directories.

Evidence gap (not a code defect). netstack_gone_mid_bind was edited (new arm shapes) and has not run on its declared target, the T14 shared boot. It is staged with --metal-readback. The queued T14 run closes this before landing.

The untested cases

  • Two processes at once. Recommended before landing, and cheap: it fits in the same netcase boot. Parent and child each block in accept (and in recv_from) at the same time, and the harness connects twice (and sends two datagrams). Each must take exactly one, and neither may hang. This is the only check that exercises the client's lose-then-wait path (waited) under a real race. That path is the "no lost wake" claim, and blocker 1's change will rewrite it. Host tests cover only the node's half.
  • A survivor after a holder is killed. No test needed now. Socket holders share no memory, gate or region, unlike streams. To netstack, a killed holder is the same connection EOF as an exit, and kernel message_send is all-or-nothing, so a kill cannot leave half a frame. Netstack's departure path is host-tested (a_listener_ends_with_its_last_holder, the datagram departure test). The issue's exit keeps the guest case.
  • Two threads in one process. Inspection suffices. TURNS is a futex lock held only around one request and its answer, never across the bell wait. Ends keeps the slot reserved until free finishes.

Suggestions

  • issues/a-socket-libc-or-socket2-has-not-bound-is-not-shared-with-a-child.md:17-21: the first bullet under "What is not shared" is a test gap, not something unshared. Move it to its own line.
  • issues/netstack-moves-every-byte-through-two-kernel-pipes.md:80: the rewritten stage 4 still plans the bound as toyos_abi::net::UDP_MAX_PAYLOAD. The bound now lives in toyos::net::socket::MAX_DATAGRAM, and a netstack protocol bound does not belong in toyos-abi.
  • sdk/std/sys/net/connection.rs:413: "a send never waits" is not quite true. send_to blocks on the IPC send and on netstack's answer. Delete the clause, or say that SO_SNDTIMEO is not honoured.
  • system/netstack/src/serve.rs:580: rename socket_id to key.
  • tests/toyos.rs:5170: the forwarded port is probed free and then released before QEMU binds it, a small race the PR body does not mention. hostfwd=…:0-:7070 with a monitor query of the chosen port would remove it. If that is not done, say at the site that the race exists.
  • sdk/toyos/src/spawn.rs:175: refuse_streams names streams only. Endowing a socket's bell directly fails as the kernel's PermissionDenied, not as a named refusal. That is harmless; optionally refuse it by name, as streams are.

SEND BACK

…eer is netstack's, and mio's comments say so

- An accept or a receive asks netstack only once its holder's bell rang, or
  once it read a ring and its request did not go (a flag per end, under the
  end's turn): an empty bell waits or answers WouldBlock with no request and
  no region. An export rings its new holder at once of what waits, so no pass
  need come before its first look.
- A datagram socket's peer is [udp]'s connected socket: a holder's CONNECT,
  PEER and SEND_PEER frames, so every holder shares the peer and its filter,
  and a connected socket's pending ICMP error rings and is answered once
  (`Refused::ConnectionRefused`, `Refused::Unreachable`). The process-local
  peer goes from toyos::net::socket and libc; libc's connect of an unbound
  datagram socket binds it, as a first send does.
- The mio fork's listener comments describe the bell (5d013b8); the lock
  names it.
- socket_export races the parent and a second child, both blocked in an
  accept and then a receive: each takes one connection and one datagram, and
  the harness checks the job's claim lines.
- The review's suggestions on the issue records, the forward's race, and a
  name in serve.rs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2: PR #1052 at 2222561 (follow-up to r1)

Architecture

Round 1's design points are resolved in the right places:

  • Blocker 2 (UDP peer). The peer now belongs to [udp]'s socket. The holder frames CONNECT, PEER and SEND_PEER go onto Udp::connect, binding and send, so every holder shares the peer and its receive filter. End::peer, Ends::update, libc's SocketEntry::peer and std's copy are all deleted. libc's connect on an unbound datagram socket binds at 0.0.0.0:0 and connects. On failure it lets go of the bound socket and leaves the entry unchanged. a_connected_sockets_peer_is_every_holders establishes the node half.
  • Frame numbers. Holder frames are now 1–7, 13–15 and 0x81–0x84. A new connection's frames are 8, 12, 23 and 25, so they do not overlap, and a_new_connection_names_no_socket includes the three new numbers. asked() requires CONNECT's exact Datagram size and a bare PEER, and caps SEND_PEER at MAX_DATAGRAM with no handles. Wrong-kind and wrong-shape frames are rejected in the boundary test.
  • Blocker 3 (mio). Fork commit 5d013b8 has 5c122cf as its only parent. The toyos branch is 1 ahead and 0 behind the old pin, so it is append-only. The change is a rename (wakes to bell) plus two corrected comments, and is upstream quality. Cargo.lock changes only mio's line. The missing Co-Authored-By trailer cannot be fixed on an append-only branch, and it does not block.
  • Blocker 1 (region per look). Fixed for the cost it named: a look with an empty bell makes no region and sends no request. The change introduces the readiness defect in finding 1.

Size. This round is +430/−153.

Findings

BLOCKER 1: sdk/toyos/src/net/socket.rs:213-226 (look) with :173 (RUNG). A ring kept by a failed look is invisible to every poller, so a readiness-driven server can stall permanently.

  • What changed. look drains the bell before stream::accept reserves a stream slot (stream.rs:815) and creates the region (:817). If either step fails (ResourceExhausted from a full stream table or strict commit), the ring survives only in RUNG.
  • Why it is lost to pollers. The descriptor's handle is the bell, and the module doc (:27-28) says a poller watches the bell for reading. mio's selector does, and so does libc's poll/ppoll (posix_io.rs wait_fds). With the bell now empty, the holder is never reported readable again. Netstack will not ring it either, because the holder was told and has not asked since. That holds even as more connections arrive.
  • Consequence. A server that responds to EMFILE/ENOMEM from accept by going back to poll never wakes on that listener again. Nginx does exactly that: it disables accept events on EMFILE and re-arms them later. It is the server the owner's ruling names. On Linux the listener stays level-readable. Retry-without-wait callers (std's blocking accept, tokio's cached readiness) are unaffected.
  • Regression against r1. At f12211e the drain sat inside open's closure, after the slot and region, so a failure there left the ring in the bell.
  • The doc is now false for readiness callers. "A ring read by a call that failed before its request went is kept for the next, so none is lost" (:23-25) does not hold for them.
  • Suggested fix. Keep the ring in the bell until the request goes:
    • Test the bell's readability without consuming it. A zero-timeout wait on it does this; look already builds a Poller for its wait.
    • Make the slot and region only when it is readable.
    • Drain immediately before the request, as r1 did.
    • This deletes RUNG, its resets in open and adopted, and the asked callback.
    • Any other design that keeps a pre-request failure visible to readiness is fine.
  • On your question about a test. No test is needed for the flag, because this fix removes it. Inspection then suffices: the only state is the bell, and only this end's look reads it, under its turn.

BLOCKER 2: system/netstack/node/src/datagram.rs:204-210 (udp_recv_from). An error answer leaves the asker untold, so a datagram waiting behind a pending ICMP error is stranded. The PR's "answered once" claim has no test.

  • The path. holders.asked(holder) clears the asker's told state. Then self.stack.recv_from(...).map_err(refused)? returns early, before tell_datagram_holders(). Until this round that path was unreachable (Failed was unreachable!). CONNECT makes it live: Udp::recv returns the pending error first (udp/src/lib.rs:595) and leaves rx intact.
  • Consequence. Take a connected socket with one holder, a datagram from the peer in rx, and then an ICMP port-unreachable for our send. The holder is rung, its receive is answered ConnectionRefused exactly once, and it is not rung again. Its next recv_from finds the bell empty and waits, or answers WouldBlock and is never readable to a poller. That lasts until some unrelated pass (network receive, timer or stream wake) calls tell_holders.
  • Why this breaks the law. The node's own rule is to ring again after any ask, before the answer goes. The serve loop does not call bridge after socket frames (serve.rs:641 sets the flag only for streams), so nothing covers the gap.
  • Fix. Tell the holders whatever the result, e.g. let received = self.stack.recv_from(id.0, out); self.tell_datagram_holders(); received.map_err(refused)?.
  • Evidence gap. The PR states "a connected socket's pending ICMP error rings its holders and is answered once". No test delivers an ICMP error to a node-held socket: git grep finds no ConnectionRefused or Unreachable in system/netstack/node/tests/.
  • The cheapest sufficient check is one node test in tests/datagram.rs:
    • two holders of a connected socket, a datagram from the peer queued, then an ICMP port-unreachable that quotes the socket's flow;
    • both holders are rung;
    • one udp_recv_from gets Refused::ConnectionRefused;
    • that asker is rung again and takes the datagram, or the other holder does;
    • a second receive does not see the error again.
  • Negative control. Before the fix, the "asker rung again" assertion is the one that goes red.

Checked and fine

  • The two-process test. After the sequential child, the parent and a racer each run race. Each process accepts exactly once and receives exactly once. The harness requires two connections, both closed with the line and each within CEILING, and checks both claim lines. The parent asserts that the two datagrams it collected sort to ["one","two"]. A lost wake therefore shows up as a ceiling failure, and a process that took both cannot pass. The test does not force the loser path; it allows it. That is acceptable for a smoke test of the race; the node half is host-tested. The guest log guest-r1b.log passes at this head.
  • Mutation m3. Dropping the export's ring turns a_holder_made_while_a_connection_waits_is_rung_at_once red, and the evidence log shows it.
  • Departure, export and new numbers. export_listener and export_datagram tell after adding the holder. The datagram counterpart is a_holder_made_while_a_datagram_waits_is_rung_at_once.

Suggestions

  • system/netstack/src/serve.rs:233-234: "The pipe ABI's word" names the wrong thing, because this is toyos::net's protocol. ERR_OTHER becomes EIO and Other, where POSIX callers expect EHOSTUNREACH. Optional now. If kept, shorten the comment to say that no finer code exists yet.
  • issues/a-socket-libc-or-socket2-has-not-bound-is-not-shared-with-a-child.md:17-20: "both blocked at once" is stronger than the test. Each racer says it is about to block before it does. "both waiting on one queue" would be accurate.

SEND BACK

…eive's error rings again

- A look reads its bell's readiness without reading it, makes the stream's
  slot and region only when it rang, and reads the bell empty just before the
  request goes. A look that fails before its request leaves the ring where
  mio and libc's poll see it; the in-process kept-ring flag goes.
- A datagram receive tells the holders whatever it answers: a pending ICMP
  error answered once no longer strands the datagram behind it.
  `a_pending_error_is_answered_once_and_the_datagram_behind_it_rings_again`
  holds it and was red before the fix; the ICMP builder moves to the tests'
  `lan` module for it.
- The review's two wording suggestions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Review r3: PR #1052 at 032114b (follow-up to r2)

Architecture

This round fixes both blockers and leaves the design unchanged. The client-side state is now just the bell. RUNG, its two resets and the asked callback are gone, and the only readiness signal left is the one pollers already watch. The node change is three lines. Production is roughly −20 lines net (socket.rs, stream.rs, datagram.rs). Test growth is the new node test, and the ICMP builder moves from resolve.rs to the shared lan module, where two test crates now use it.

Blocker 1 (kept ring lost to pollers): resolved

sdk/toyos/src/net/socket.rs:202-235. look takes the end's turn and calls rung(bell, 0), which reads nothing. It calls ask only while the bell is readable.

  • Before drain (accept). The slot reservation, SharedMemory::create and share (stream.rs:815-826) all run before drain (stream.rs:752). Each failure returns ResourceExhausted or Io with the bell still readable, so a poller (mio, libc poll) still sees the listener.
  • Receive. recv_from (socket.rs:384-386) drains immediately before signal(RECEIVE).
  • After a request goes. The node rings again after every ask when something still waits (listeners.rs:188-190 passes, and datagram.rs now always tells). A ring the drain cleared is therefore replaced by the node.
  • Zero timeout. inbox_submit with timeout 0 is non-blocking (kernel/src/inbox/mod.rs:383-405). polls::deliver runs before that return, so the result is the current level.
  • No spin. look loops only when ask answers WouldBlock. Every WouldBlock comes after drain, either from netstack's answer or after the request, so the next pass sees the bell empty and blocks in rung(bell, left). A failure before drain returns to the caller and does not loop. A hung-up bell makes the request fail with NetstackNotFound.
  • No stall. A ring between rung(bell, 0) and the blocking rung(bell, left) is seen, because the watch is level-triggered.

Blocker 2 (ICMP error stranding a datagram): resolved

system/netstack/node/src/datagram.rs:208-211. The asker's state is now told before either result is answered. listeners.rs accept already did this, because it calls pass before returning. udp_recv_from was the only other caller of asked.

  • The test (node/tests/datagram.rs:293-317) follows the requested shape and is focused: two holders, a datagram queued, then an ICMP port-unreachable. Both holders are rung once, ConnectionRefused is answered once, the asker is rung again, and the other holder receives the datagram rather than the error.
  • Negative control. r2-before-fix.log shows it red on exactly the asker-rung-again assertion (left: 1, right: 2), and ci-host-r2.log:4174 shows it green.

Evidence

  • Host run. ci-host-r2.log ends at 1 of 103 steps red. The red step is clippy, with a single error: unused sum at node/tests/resolve.rs:16. That error is a product of moving the builder, so the run covered this round's code apart from the one-line fix.
  • Other failures in the log. The other FAILED lines are the negative controls reaching their verdicts (transport-ungated-violation and others), not red steps.
  • Fix check. The fix deletes that one identifier from one test target's import list. cargo clippy -p toyos-net-node --all-targets -- -D warnings exiting 0 is sufficient. The log does not record its own command, but the PR body does, and inspection settles it anyway: deleting an unused import cannot change any other step. No full --ci host rerun is needed.
  • Guest. netstack_holders_exported_to_children, libc_children and iommu_virtio_platform pass in guest-r2.log. stream_holders passes alone in guest-r2b.log.
  • The wedged first run. The combined run's stream_holders worker waited on the sysroot lock held by its own process (pid 73228). This round touches no harness code, and the PR touches no lock code. The failure is already recorded in issues/a-suite-that-builds-its-sysroot-can-wedge-on-its-own-locks.md. It is independent of this change.
  • T14. netstack_gone_mid_bind uses bind and connect only, never look, so the result at 2222561 stands.

Findings

No blockers.

SUGGESTION: sdk/toyos/src/net/socket.rs:203 (rung) builds a Poller on every look.

  • Cost. rung now makes a Poller on every accept and receive, including non-blocking ones. That is inbox_setup (a page and its mapping), a submit and a close per call, where r2 had one read_nonblock. This is still far below main's connection per request.
  • Panic. It also means a non-blocking recv_from under memory exhaustion panics in Poller::new's expect instead of answering ResourceExhausted. That was the wait path's existing behaviour, and it now reaches every look.
  • Possible fix. Make one Poller per end when the end is made, and use it under the turn. This would remove both the cost and the panic. It is not required for this PR.

SUGGESTION: sdk/toyos/src/net/socket.rs:24-25 overstates "none is lost".

  • The window. The send itself can be refused after drain: the kernel's SendRefused::Error(ResourceExhausted) on PipeWrite::NoMemory, kernel/src/object/service.rs:60. That consumes the ring without a request, and hangup maps the refusal to Io.
  • Why it does not block. The window is kernel memory exhaustion only. QueueFull cannot occur, because the turn allows one request per end. The order cannot be reversed safely: draining after the send could swallow the node's re-ring.
  • Possible fix. Narrow the sentence to failures before the send, or note the refused-send case.

LAND

A look made and closed a kernel poller each time, so under memory
exhaustion a nonblocking receive or accept panicked in `Poller::new`
instead of answering. Each end now makes its poller with itself, held in
its turn, and a look that does not wait asks the kernel for nothing it can
refuse; making an end refuses `ResourceExhausted` where the kernel makes no
poller. A blocking wait, which runs outside the turn the end's other calls
take meanwhile, makes its own with `Poller::try_new` and answers
`ResourceExhausted` where it cannot. The module's no-lost-ring sentence now
names what it covers: a call that fails before its request goes.

`issues/a-suite-that-builds-its-sysroot-can-wedge-on-its-own-locks.md`
records round 2's recurrence of that harness wedge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Landing at e5f90fd. Review r3 returned LAND, and I required one last change: each socket end makes its poller with the end, so a non-blocking call can no longer panic under memory exhaustion (a blocking wait uses Poller::try_new and answers ResourceExhausted). Full host CI is 103 of 103 green, and the netcase rows pass. T14 netstack_gone_mid_bind passed at 2222561; this round does not touch its path. This closes issues/netstack-socket-ids-are-ambient.md.

@Japabu
Japabu marked this pull request as ready for review October 23, 2026 22:20
@Japabu
Japabu added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 0587323 Oct 23, 2026
8 checks passed
@Japabu
Japabu deleted the wt/netstack-socket-holders branch October 23, 2026 23:01
Japabu added a commit that referenced this pull request Oct 24, 2026
The T14's ToyOS osbench boot at f644cef killed `osbench lan` at the list
bound, 321 s into a group whose discovery is bounded at 60 s: with no peer
on the link its `recv_from`, under a 1 s read timeout, never returned
(issues/std-udp-socket-ignores-its-timeouts.md). That was netstack parking
a datagram receive until a datagram came. #1052 has since made a datagram
socket its holder's own and std's receive wait on it with the read
timeout, which is that issue's fix; its exit asked for a guest test with no
answering peer, and this is it.

`udp_receive_times_out`, on netcase's real netstack: two receives under a
300 ms timeout with nothing sent each end `WouldBlock`, std's Unix word for
a receive timeout, after at least 300 ms and inside a 30 s hang ceiling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant