close
Skip to content

std/sys/net/xous: read NetError code from byte 4 in recv/accept paths#156414

Open
tunnell wants to merge 1 commit into
rust-lang:mainfrom
tunnell:pr-xous-net-recv-byte-offset
Open

std/sys/net/xous: read NetError code from byte 4 in recv/accept paths#156414
tunnell wants to merge 1 commit into
rust-lang:mainfrom
tunnell:pr-xous-net-recv-byte-offset

Conversation

@tunnell
Copy link
Copy Markdown

@tunnell tunnell commented May 10, 2026

The Xous std backend's TCP send path reads the NetError
code from send_request.raw[4] (correct — that's where the
xous-core kernel's respond_with_error writes it). But the TCP
recv, UDP recv, and TcpListener accept paths all read from
raw[1] instead. The kernel's historical layout is
[1, 1, 1, 1, code, 0, 0, 0], so byte 1 is always 1 and
ErrorKind::TimedOut / ErrorKind::WouldBlock are unreachable
from the recv side — every error falls through to the catch-all
ErrorKind::Other("recv_slice failure").

This patch moves the recv-path checks to raw[4], matching the
send path and the kernel's existing encoding. Three files,
identical one-byte change in each (+15 / -10 total). No new
behavior; an existing inconsistency between the send and recv
sides of the same backend is removed.

Why this matters in practice: any application using
set_read_timeout() to interleave reads and writes on a Xous
TcpStream (e.g. a tungstenite-based WebSocket pump) currently
can't distinguish "no data this poll" from real transport
failure. Concretely, a Signal client using a 5s read timeout saw
every WebSocket torn down within 5s of opening, because the
timeout was indistinguishable from a fatal error.

Coordination with the kernel side: a complementary kernel-
side change has been filed on the xous-core repo —
betrusted-io/xous-core#877
— which mirrors the code at byte 1 in addition to byte 4 so
applications work immediately on stock Rust toolchains while
this PR cycles. After both land, the two sides agree at byte 4
and byte 1 stays mirrored only for backwards-compat with older
Rust. Either change alone fixes the user-visible symptom; both
together remove the wire-format ambiguity.

Tests: Tier-3 target std backends generally don't have CI
coverage in this repo, so I haven't added any. Happy to add an
in-process test that constructs a fake net-server response and
asserts the ErrorKind mapping if maintainers prefer.

Full disclosure: I used an AI agent to debug this problem and eventually track it here.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 10, 2026
@tunnell tunnell marked this pull request as ready for review May 10, 2026 19:51
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 10, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 10, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 10, 2026

r? @nia-e

rustbot has assigned @nia-e.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ChrisDenton, libs
  • @ChrisDenton, libs expanded to 8 candidates
  • Random selection from Mark-Simulacrum, nia-e

@rustbot

This comment has been minimized.

@bjorn3
Copy link
Copy Markdown
Member

bjorn3 commented May 11, 2026

cc @xobs as target maintainer

@xobs
Copy link
Copy Markdown
Contributor

xobs commented May 11, 2026

This fix looks good to me. Thanks for tracking it down, @tunnell .

Comment thread library/std/src/sys/net/connection/xous/tcplistener.rs Outdated
Comment thread library/std/src/sys/net/connection/xous/udp.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 12, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@nia-e
Copy link
Copy Markdown
Member

nia-e commented May 12, 2026

Hi - see the comment from rustbot. Otherwise, I will defer to @xobs's judgement since the code change looks okay with some tiny style nitpicks

@tunnell
Copy link
Copy Markdown
Author

tunnell commented May 16, 2026

Thanks for the quick feedback and glad this is useful.

I implemented the style changes (that were pre-existing issues?) as individual commits in case you want to squash merge.

@rustbot ready.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 16, 2026
`respond_with_error` in xous-core's `services/net/src/std_glue.rs`
writes the `NetError` code at byte 4 of the 8-byte response
buffer. The send path in `tcpstream.rs::write` correctly reads
`send_request.raw[4]`. The recv-side decoders, however, read
`result[1]` / `raw[1]`, which under the historical
`[1, 1, 1, 1, code, 0, 0, 0]` layout is always `1` — so
`ErrorKind::TimedOut` and `ErrorKind::WouldBlock` were
unreachable from the recv side.

Three files affected, identical pattern:

  - `tcpstream.rs::read_or_peek` (recv decode)
  - `udp.rs` (UDP recv decode)
  - `tcplistener.rs` (accept decode)

Each `result[1]` / `raw[1]` becomes `result[4]` / `raw[4]`,
matching the send path. Behavior is unchanged for any kernel
that respects the existing 8-byte error layout.

A complementary kernel-side change in betrusted-io/xous-core
(betrusted-io/xous-core#877) mirrors
the code at byte 1 too, so existing toolchain versions also
see the right value immediately. The two changes are non-
conflicting.

Co-authored-by: Nia <nia-e@haecceity.cc>
@tunnell tunnell force-pushed the pr-xous-net-recv-byte-offset branch from 3b3b1c1 to 4ec7394 Compare May 16, 2026 11:52
@tunnell
Copy link
Copy Markdown
Author

tunnell commented May 16, 2026

Squashed into one commit and reworded the body to use a full URL instead of #877 — should clear the rustbot warning. Had to force push.

@rustbot ready.

Thanks, @nia-e!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants