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.

`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` (rust-lang#877) mirrors the code at byte 1 too,
so existing toolchain versions also see the right value
immediately. The two changes are non-conflicting.
@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
Copy link
Copy Markdown
Collaborator

rustbot commented May 10, 2026

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

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

if receive_request.raw[0] != 0 {
// error case
if receive_request.raw[1] == NetError::TimedOut as u8 {
// error case — code lives at byte 4 (where the send path
Copy link
Copy Markdown
Member

@nia-e nia-e May 12, 2026

Choose a reason for hiding this comment

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

Suggested change
// error case — code lives at byte 4 (where the send path
// Error case — code lives at byte 4 (where the send path

View changes since the review

if receive_request.raw[0] != 0 {
// error case
if receive_request.raw[1] == NetError::TimedOut as u8 {
// error case — code lives at byte 4 (where `send_message`
Copy link
Copy Markdown
Member

@nia-e nia-e May 12, 2026

Choose a reason for hiding this comment

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

Suggested change
// error case — code lives at byte 4 (where `send_message`
// Error case — code lives at byte 4 (where `send_message`

View changes since the review

@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

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

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants