Skip to content

fix(test): block accepted sockets so Windows test servers stop flaking - #1386

Merged
zackees merged 1 commit into
mainfrom
fix/windows-accepted-socket-inherits-nonblocking
Aug 23, 2026
Merged

fix(test): block accepted sockets so Windows test servers stop flaking#1386
zackees merged 1 commit into
mainfrom
fix/windows-accepted-socket-inherits-nonblocking

Conversation

@zackees

@zackees zackees commented Aug 23, 2026

Copy link
Copy Markdown
Member

Symptom

Check (windows-latest) failed on #1385, in a test that PR does not touch:

usb::data::tests::populate_online_cache_falls_back_to_json_when_proto_is_missing ... FAILED
thread '<unnamed>' panicked at crates\fbuild-core\src\usb\data.rs:643:55:
called `Result::unwrap()` on an `Err` value: Os { code: 10035, kind: WouldBlock,
  message: "A non-blocking socket operation could not be completed immediately." }

(The second panic, Any { .. } at data.rs:680, is just handle.join().unwrap()
resurfacing the server thread's failure.)

Cause

Both test servers set their listener non-blocking and correctly handle
WouldBlock around accept(). What neither handled:

On Windows, an accepted socket inherits the listening socket's
non-blocking mode. On Unix, accept() returns a blocking socket regardless.

So stream.read(&mut buf).unwrap() on the fresh connection returns
WSAEWOULDBLOCK whenever the client's bytes have not arrived in the microsecond
between accept and read. Nothing about the failure points at socket
mode — it lands in a test about USB VID overlay fallback, on Windows only,
intermittently.

os error 10035 is WSAEWOULDBLOCK, which is why kind is WouldBlock rather
than something that reads as a connection problem.

Fix

Put the accepted socket back into blocking mode, which is what the surrounding
read/write sequence already assumes.

This is not a new pattern — crates/fbuild-cli/tests/test_emu_exit_code.rs's
mock daemon has done exactly this since it was written:

stream.set_nonblocking(false).expect("blocking per-connection");

The two servers fixed here just never got the same treatment.

Scope

  • crates/fbuild-core/src/usb/data.rs — the one that failed
  • crates/fbuild-cli/src/cli/port_scan.rs — same shape, same latent flake

crates/fbuild-core/src/usb/profiles.rs has an identically-configured listener
but only counts accepts (Ok(_) => extra_requests += 1) and never reads from
the stream, so it cannot hit this. Left alone rather than changed for symmetry.

Verified: usb::data 8 passed, port_scan 25 passed, clippy -D warnings
clean. A timing flake can't be proven fixed by one green run — the argument is
the mechanism, and it's the same one the working mock daemon already relies on.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 54 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 415376e6-b9d4-4276-ae83-0c23dc50a68e

📥 Commits

Reviewing files that changed from the base of the PR and between 9f82da3 and b8c2b59.

📒 Files selected for processing (2)
  • crates/fbuild-cli/src/cli/port_scan.rs
  • crates/fbuild-core/src/usb/data.rs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

`Check (windows-latest)` failed on an unrelated PR with:

    usb::data::tests::populate_online_cache_falls_back_to_json_when_proto_is_missing
    thread '<unnamed>' panicked at crates\fbuild-core\src\usb\data.rs:643:55:
    called `Result::unwrap()` on an `Err` value: Os { code: 10035,
      kind: WouldBlock, message: "A non-blocking socket operation could not
      be completed immediately." }

Both test servers set their listener non-blocking and handle `WouldBlock`
around `accept()`. What neither handled is that **Windows gives back an
accepted socket that inherits the listener's non-blocking mode**, where Unix
gives back a blocking one. So `stream.read(..).unwrap()` on the fresh
connection returns WSAEWOULDBLOCK whenever the client's bytes have not landed
in the microsecond between accept and read — a timing-dependent failure whose
message points at sockets rather than at the socket *mode*, on a test about
USB VID overlays.

The accepted socket is now put back into blocking mode explicitly, which is
what the loop already assumes. `test_emu_exit_code.rs`'s mock daemon has
guarded this way from the start ("blocking per-connection"); these two just
never got the same treatment.

`usb/profiles.rs` has the same listener shape but only counts accepts and
never reads from the stream, so it is unaffected and left alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zackees
zackees force-pushed the fix/windows-accepted-socket-inherits-nonblocking branch 2 times, most recently from 3db5364 to b8c2b59 Compare August 23, 2026 19:52
@zackees
zackees merged commit 1a36249 into main Aug 23, 2026
95 of 97 checks passed
@zackees
zackees deleted the fix/windows-accepted-socket-inherits-nonblocking branch August 23, 2026 20:17
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant