Skip to content

fix(wasix): de-flake udp-large-recv and stream-tcp-writev-partial tests - #6789

Open
Arshia001 wants to merge 4 commits into
mainfrom
fix/flaky-udp-tcp-socket-tests
Open

fix(wasix): de-flake udp-large-recv and stream-tcp-writev-partial tests#6789
Arshia001 wants to merge 4 commits into
mainfrom
fix/flaky-udp-tcp-socket-tests

Conversation

@Arshia001

Copy link
Copy Markdown
Member

Summary

Fixes the two flaky socket integration tests reported in #6785. Both flaked under nextest (worse on macOS and under concurrent load) — no runtime code is changed, only the tests.

udp-large-recv

The test sent a single 20480-byte UDP datagram over loopback and did one blocking recvfrom. UDP is best-effort: a dropped datagram left recvfrom to wait out the socket's default 30s read timeout and return -1 (ETIMEDOUT) — the "gets stuck ~1 min then fails" symptom. This is common on macOS (default net.inet.udp.maxdgram is 9216, and loopback drops large datagrams under load) and under nextest's concurrent-process load.

Now it:

  • retries the send/receive a few times, each bounded by poll(POLLIN, 1s) (wasix-libc doesn't wire SO_RCVTIMEO, so poll() is used, matching the udp-readiness test);
  • treats a failed sendto (e.g. macOS EMSGSIZE on oversized datagrams) as a dropped attempt;
  • skips (exit 0) if no datagram is ever delivered — a drop is not the behaviour under test;
  • fails only when a datagram is delivered but has the wrong length or payload (truncation / sharding / corruption), which is the real invariant.

stream-tcp-writev-partial

The test relied on the peer's RST landing in the tiny window between the two back-to-back internal send() calls of a single writev. That transition is an asynchronous RST round-trip, so depending on timing the syscall returned the first-iovec length (pass), the full length, or -1 — inherently racy. Virtual SO_SNDBUF/SO_RCVBUF tuning is a no-op on host sockets, so the buffer can't be shrunk to make the boundary controllable either.

Rewritten to exercise the same fd_write contract — "return bytes already transferred instead of failing the whole syscall" — via a deterministic short write: client is non-blocking, server never reads, and writev([5 bytes, 64 MB]) sends the small iovec in full and short-writes the oversized one, so the loop returns a partial total. A whole-syscall failure (the regression) would surface as -1 and fail the assertion.

Caveat: this guards the local_sent != buf.len() => break branch rather than the exact Err(_) if sent > 0 => break arm the original regression fixed — same user-visible contract, different line. Deterministically hitting the error arm from guest code isn't possible (it needs an async error mid-syscall); guarding that specific line would require a runtime-side unit test against a mock VirtualTcpSocket.

Testing

  • udp-large-recv: happy path passes; forced persistent drops skip in ~8s (exit 0, no 30s hang); forced short delivery fails as expected. Verified on default and v2026-05-12.1 libc (cranelift).
  • stream-tcp-writev-partial: 6/6 deterministic passes on both libc variants; confirmed it produces a genuine multi-MB partial.

Closes #6785

🤖 Generated with Claude Code

Both socket integration tests were flaky under nextest (issue #6785),
timing out / returning wrong values under load, primarily on macOS.

udp-large-recv relied on reliable delivery of a single 20480-byte UDP
datagram over loopback. UDP is best-effort, so a dropped datagram left
the blocking recvfrom to wait out the socket's 30s read timeout and
return -1. Retry the exchange a few times with a poll()-bounded wait
(wasix-libc does not wire SO_RCVTIMEO), skip (exit 0) if no datagram is
ever delivered, and only fail on truncation/sharding/corruption when a
datagram does arrive.

stream-tcp-writev-partial relied on the peer's RST landing in the window
between two back-to-back internal send() calls of a single writev, which
is inherently racy (returns first-iovec len, full len, or -1 depending
on timing). Rewrite it to exercise the same "return bytes already
transferred" fd_write contract via a deterministic short write: a tiny
first iovec followed by an oversized second iovec that can never fully
fit the send buffer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 9, 2026 08:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates two WASIX socket integration tests to reduce flakiness under nextest/concurrent load, without changing runtime behavior. It aims to avoid long hangs on dropped UDP datagrams and to make the TCP writev partial-write assertion deterministic.

Changes:

  • udp-large-recv: retry send/receive attempts with poll()-bounded waits and skip when no datagram is ever delivered.
  • stream-tcp-writev-partial: rework the test to provoke a partial writev() via non-blocking I/O and an oversized second iovec instead of racing a peer close/RST.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/wasix/tests/wasm_tests/socket/udp-large-recv/main.c Adds bounded polling + retry/skip behavior to avoid hangs on dropped large UDP datagrams.
lib/wasix/tests/wasm_tests/socket/stream-tcp-writev-partial/main.c Rewrites the test to target a deterministic short write path for writev partial-success behavior.

Comment on lines +81 to +87
int pr = poll(&pfd, 1, RECV_TIMEOUT_MS);
if (pr <= 0 || (pfd.revents & POLLIN) == 0) {
// Timed out waiting for the datagram (dropped in transit); retry.
fprintf(stderr, "attempt %d: poll timed out (pr=%d revents=0x%x)\n",
attempt, pr, pfd.revents);
continue;
}
Comment on lines +89 to +94
ssize_t nread = recvfrom(receiver, recvbuf, sizeof(recvbuf), 0, NULL, NULL);
if (nread < 0) {
fprintf(stderr, "attempt %d: recvfrom failed (errno %d: %s)\n", attempt,
errno, strerror(errno));
continue;
}
Comment on lines +141 to +145
if (written <= (ssize_t)FIRST_IOV_LEN ||
written >= (ssize_t)(FIRST_IOV_LEN + (size_t)SECOND_IOV_LEN)) {
fprintf(stderr,
"expected writev to return %d bytes after peer close, got %zd "
"errno=%d (%s)\n",
FIRST_IOV_LEN, written, errno, strerror(errno));
"expected partial writev in (%d, %zu), got %zd errno=%d (%s)\n",
FIRST_IOV_LEN, FIRST_IOV_LEN + (size_t)SECOND_IOV_LEN, written,
@Arshia001
Arshia001 requested a review from marxin July 9, 2026 09:24
The stream-tcp-writev-partial rewrite exercises fd_write's partial-return
contract through the short-write branch, but not the exact
`Err(_) if sent > 0 => break` arm the original regression fixed. That arm
requires a later per-iovec send() to error after an earlier iovec already
succeeded, which cannot be triggered deterministically over real host
sockets (it races an asynchronous RST landing between two back-to-back
sends of one writev).

Add a test-only mock VirtualNetworking whose TCP socket succeeds on the
first send() and returns ECONNRESET afterwards, and drive a small C guest
(connect + writev of two iovecs) against it via the existing runtime-config
test harness. writev must return the first iovec length. Verified it fails
(returns -1) when the partial-return arm is removed. No application code is
changed; async-trait is added as a dev-dependency for the mock impl.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Arshia001

Copy link
Copy Markdown
Member Author

Added a follow-up commit that closes the coverage gap noted above.

stream-tcp-writev-partial (the short-write rewrite) guards the local_sent != buf.len() => break branch of fd_write, but not the exact Err(_) if sent > 0 => break arm the original regression fixed. That arm needs a later per-iovec send() to error after an earlier iovec already succeeded, which can't be triggered deterministically over real host sockets (it races an async RST landing between two back-to-back sends of one writev).

The new wasm/writev_partial_send_error test drives it against a test-only mock VirtualNetworking (tests/wasm_tests/mock_net.rs) whose TCP socket succeeds on the first send() and returns ECONNRESET afterwards. A small C guest connects and writevs two iovecs; fd_write must return the first iovec length. It runs through the existing runtime-config harness path (mirroring the dynamic_runtime_hooks smoke test) and the C driver carries an Ignored directive so the auto-collected real-networking run is skipped.

  • No application code changed; async-trait added as a dev-dependency for the mock impl.
  • Verified the test fails (writev returns -1) when the Err(_) if sent > 0 => break arm is removed, and passes with it intact.

…ull writev

Incorporate PR review feedback so the de-flaked tests can still catch
regressions instead of masking them:

- udp-large-recv: distinguish a dropped datagram (poll() timeout, pr==0)
  from a genuine error. Retry on EINTR, treat pr==0 as a drop, but fail
  hard on any other poll() error or unexpected revents, and on a recvfrom()
  error after POLLIN (readiness reported then recv fails is a real bug, not
  a drop).

- stream-tcp-writev-partial: if the host's socket buffers accept the whole
  oversized write, we cannot force a short write; treat that as an
  environment limitation and skip rather than false-fail. Still fail on
  written < 0 (the regression). Drop the ExpectedStdout directive since the
  skip path no longer prints it; assert via exit code.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Arshia001

Copy link
Copy Markdown
Member Author

Thanks — addressed all three review points in bc1cf3f:

  1. udp-large-recv poll() errors — now only pr == 0 (timeout) is treated as a dropped datagram. EINTR retries; any other poll() error or unexpected revents (POLLERR/POLLHUP/…) fails hard instead of skipping, and the misleading "timed out" message is gone.
  2. udp-large-recv recvfrom() after POLLIN — a recvfrom failure once the socket is reported readable now fails the test (except EINTR), rather than retrying/skipping, so a "readiness reported then recv errors" regression can't be masked.
  3. writev 64 MiB full-write — if the host accepts the whole write (buffers ≥ 64 MiB), the test now skips ("environment can't force a short write") instead of false-failing; it still fails on written < 0 (the regression). Dropped the ExpectedStdout directive since the skip path no longer prints it and assert via exit code.

Re-verified: happy paths pass on both libc variants, the udp skip path still exits 0 (~8s) on forced drops, and the writev mock test still fails when the Err(_) if sent > 0 => break arm is removed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment on lines +66 to +75
ssize_t nsent = sendto(sender, sendbuf, PAYLOAD_SIZE, 0,
(struct sockaddr*)&addr, sizeof(addr));
if (nsent != PAYLOAD_SIZE) {
// e.g. macOS rejects datagrams larger than net.inet.udp.maxdgram with
// EMSGSIZE. That is not the behaviour under test, so treat it as a
// dropped attempt.
fprintf(stderr, "attempt %d: sendto returned %zd (errno %d: %s)\n",
attempt, nsent, errno, strerror(errno));
continue;
}
Comment on lines +120 to +127
char* big = malloc(SECOND_IOV_LEN);
if (big == NULL) {
fprintf(stderr, "malloc failed\n");
close(client);
close(server);
return 1;
}
memset(big, 'w', SECOND_IOV_LEN);
- udp-large-recv: only treat expected best-effort/oversize/transient
  sendto() failures (EMSGSIZE/ENOBUFS/EAGAIN/EWOULDBLOCK/EINTR) as a
  dropped attempt; any other errno now fails the test instead of being
  skipped, so a real sendto regression (EBADF/EINVAL/...) cannot be masked.
  A short non-negative datagram send is also treated as a bug.

- stream-tcp-writev-partial: skip (exit 0) if the 64 MiB allocation fails
  rather than hard-failing, so constrained environments don't reintroduce
  flakiness; drop the memset since only the returned byte count is asserted.

- Apply clang-format (Google style) to satisfy the Code lint CI job.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Arshia001

Copy link
Copy Markdown
Member Author

Addressed the re-review in 2bf9566 (also fixed the clang-format CI failure in the same commit):

  1. sendto masking real regressions — the retry loop now only treats expected best-effort/oversize/transient failures (EMSGSIZE/ENOBUFS/EAGAIN/EWOULDBLOCK/EINTR) as a dropped attempt. Any other errno (e.g. EBADF/EINVAL) fails the test instead of eventually skipping, and a short non-negative datagram send is treated as a bug too.
  2. 64 MiB allocationmalloc failure now skips (exit 0) instead of hard-failing, so constrained environments don't reintroduce flakiness; dropped the memset since only the returned byte count is asserted.

Re-verified all three tests pass on both libc variants and the udp skip path still exits 0 (~8s) under forced drops. clang-format (Google style) now passes locally.

}
}));

tests.push(libtest_mimic::Trial::test(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't like this (apparently second one) special-case. If have have a special need from the harness, we should introduce a new directive and annotate the corresponding expectations in the test-case.

@@ -0,0 +1,175 @@
//! A mock networking backend used by the `writev_partial_send_error` test.
//!
//! It hands out a TCP socket whose first `try_send` succeeds in full and whose

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well, this only covers one particular scenario - it's not any generic mocking harness. How much do we benefit from the specific scenario?

@marxin

marxin commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

For unblocking of the CI on main, I temporarily disabled the test in 503f53f.
Please enable it again as part of this PR.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

udp-large-recv test gets stuck when run under nextest

3 participants