cluster: fix two ways a Windows worker stops reacting (shared accept() race, early handle ack); fixes test-cluster-shared-leak timeouts by robobun · Pull Request #37815 · oven-sh/bun

robobun · 2026-08-12T12:13:09Z

Problem

test/js/node/test/parallel/test-cluster-shared-leak.js times out on the Windows lanes on most builds since cluster: port Node's cluster and child_process handle-passing suites (+43 upstream tests; cluster 54 → 85) and implement what they expose — round-robin fd handoff, SCHED_NONE shared handles, UDP clustering, IPC handle passing #31829 (626034f), and test-cluster-disconnect.js occasionally; Linux and macOS are unaffected.
In both, a cluster worker stops reacting to IPC, so it never acts on disconnect() and the primary waits for it forever. Two separate Windows-only causes.
Cause 1, shared listening (SCHED_NONE): every worker holds a copy of one listening socket and Windows wakes all of them per connection. The loser blocks inside accept() although its copy is non-blocking (WSAAccept <- bsd_accept_socket <- uv_run) and its loop never runs again. Two idle workers also burn about 1.0 to 1.6s CPU per 3s, their exclusive AFD polls completing each other.
Cause 2, round-robin handoff: on Windows the worker's ack for a handed-off connection can be read before the write callback for that message runs. The primary dropped the ack, then parked the message as waiting for one, and every later message to that worker (pings, act: 'disconnect') queued behind it forever. Needs a busy machine; unreachable on POSIX, where the write is synchronous.

Fix

On Windows the primary now hands connections out itself (a round-robin handle) for every TCP listen, whatever cluster.schedulingPolicy says, so exactly one process accepts on any listener. On POSIX the decision reduces to the old SCHED_RR check. UDP and TLS handles still share, so two TLS workers on one Windows port stay exposed to cause 1 (pre-existing, tracked separately).
The refusal that stops a plain worker joining a TLS worker's shared socket uses the same decision, so it now also applies under SCHED_NONE on Windows; its hint no longer names SCHED_RR.
An ack or nack read while the head of the send queue is a handle message still being written is kept and applied once that write completes (a kept nack takes the normal retransmit path). It can only apply to the message it was read for, because the queue head cannot change while a write is in progress.
Verification: a new Windows-only test pushes 100 connections through two workers on one port under each policy, pinging both after each. The SCHED_NONE variant fails 10/10 unfixed; the SCHED_RR variant only failed unfixed under load (it is what caught cause 2, in this PR's first CI run). On a Windows debug build test-cluster-shared-leak.js passes 40/40 (3/40 hung unfixed) and all 85 vendored test-cluster-* files pass, on Linux too.

Background

node:cluster has two ways to put workers on one port. SCHED_NONE (a shared handle) gives every worker the listening socket and each accepts for itself; SCHED_RR (a round-robin handle) has the primary listen and pass each accepted connection to a worker over IPC. Bun already defaults to round-robin on Windows; this PR changes what an explicit SCHED_NONE gets there.
Shared listening on Windows, added in cluster: port Node's cluster and child_process handle-passing suites (+43 upstream tests; cluster 54 → 85) and implement what they expose — round-robin fd handoff, SCHED_NONE shared handles, UDP clustering, IPC handle passing #31829, duplicates the socket into each worker (WSADuplicateSocketW) and drives it with uv_poll plus a plain accept(). libuv and node drive a shared listening socket with AcceptEx, which Bun's listen sockets do not use, so node has neither cause 1 problem.
A handle message is an IPC message carrying a socket. The sender holds it as waiting for an ack until the receiver confirms it and sends nothing else on that channel until then, so a lost ack blocks the channel. The message only becomes "waiting" in the write-completion callback.
On Windows uv_write completes asynchronously, so the peer's reply can be read before the write callback runs; on POSIX the pipe write is synchronous and the callback always runs first. That is why cause 2 exists on one platform only.
A TLS listen is a sharedOnly handle: Bun's TLS accept is native and cannot adopt a connection handed over by the primary, so mixing TLS and plain workers on one port is refused with EINVAL whenever the plain side would be round-robin.

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/cluster.test.ts

Original description

Fixes test/js/node/test/parallel/test-cluster-shared-leak.js timing out on the Windows lanes since #31829 landed (626034f). It shows up on most builds since then, e.g. https://buildkite.com/bun/bun/builds/93004 (4/4 attempts on Windows 2019 x64, with a one-file diff) and https://buildkite.com/bun/bun/builds/93005 (Windows 11 aarch64); nothing on Linux or macOS. Culprit: #31829, which added the TCP SharedHandle this test exercises and the round-robin handle passing it now falls back to. Two separate Windows problems, both of which leave a worker that never reacts to disconnect() again, so the primary waits for it forever.

1. Shared listening sockets: the losing worker blocks inside `accept()`

On a Windows box with the 626034f canary, the test hangs 3/40 runs in a loop (7-15% in other batches), with the primary and both workers still alive. An instrumented copy shows that the worker which did not get the test's one connection stops processing anything from that moment on. Native stacks of its main thread:

ntdll.dll!NtWaitForSingleObject
mswsock.dll!...
WS2_32.dll!WSAAccept
WS2_32.dll!accept
bun.exe  (bsd_accept_socket <- us_internal_dispatch_ready_poll <- poll_cb)
bun.exe!uv__fast_poll_process_poll_req
bun.exe!uv_run

#31829 implements SCHED_NONE on Windows by WSADuplicateSocketW-ing one listening socket into every worker, where us_socket_group_listen_fd puts it under a uv_poll and the normal accept loop calls accept(). With more than one process on the socket:

AFD reports an incoming connection to every process polling the socket, so both workers call accept(). One gets it; the other blocks inside accept() although its descriptor is non-blocking (checked with a small bun:ffi program: the blocking mode of a duplicated socket is per descriptor, and each worker does set its own copy non-blocking in uv_poll_init_socket; accept() blocked anyway). Nothing else connects in this test, so that event loop is gone for good; in a real server it would sit there until the next connection arrives and then take it.
uv_poll submits AFD polls with Exclusive = TRUE, which completes any other outstanding exclusive poll on the same socket, including one from another process, and libuv resubmits on an empty completion. Two idle SCHED_NONE workers sharing a port burn about 1.0s and 1.6s of CPU per worker per 3s of idling (process.cpuUsage()); a single worker uses 0.

Node has neither problem because libuv's shared listening sockets (UV_HANDLE_SHARED_TCP_SOCKET) are driven with AcceptEx, which Bun's listen sockets do not use.

Fix: queryServer (internal/cluster/primary.ts) creates a RoundRobinHandle for TCP listens on Windows regardless of cluster.schedulingPolicy, so exactly one process accepts on a listener, the same path Bun already defaults to on Windows (and the PR already takes Windows-specific routes for UDP and pipes). The choice is factored into usesRoundRobinHandle(message); the existing "a plain worker may not join a TLS worker's shared-only handle" check uses the same predicate, so it also applies under SCHED_NONE on Windows (otherwise the plain worker would accept on the TLS worker's socket, which is exactly the situation above). The hint attached to that error no longer names SCHED_RR, since the condition is no longer tied to it. On POSIX the predicate reduces to the previous schedulingPolicy === SCHED_RR with the same sharedOnly/UDP exclusions, so nothing changes there; cluster.schedulingPolicy itself is not touched.

sharedOnly (TLS) handles still share on Windows, since TLS servers cannot adopt handed-off connections; two or more TLS workers on one port there remain exposed to both problems until listen sockets can accept with AcceptEx. That is pre-existing from #31829 and tracked separately.

2. Round-robin handoff: a handle ack read before the write callback was dropped

The first CI run of this PR failed its own new test on Windows 2019 x64 with the primary distributing connections: worker 2 stopped answering after connection 52, passing on retry. Reproducible on the canary too by running the round-robin variant of the fixture in 10-12 parallel copies (4/10 release, 5/12 debug); it never reproduces on an idle machine. In the stalled state the worker is idle in uv_run, the primary's worker.send() returns false, and a probe in SendQueue::on_ack_nack reports, in every stalled instance:

early ack: write_in_progress=true queue_len=1 head_has_handle=true head_cursor=0

i.e. the worker's NODE_HANDLE ack for the newconn message arrived (and was processed) before libuv had invoked the write callback for that message. Writes are asynchronous on Windows (uv_write), and it is on_write_complete that moves a handle message into waiting_for_ack; on_ack_nack therefore found nothing waiting and dropped the ack, the write callback then parked the message in waiting_for_ack, and every later message to that worker (pings, act: 'disconnect') queued up behind it forever. On POSIX the write is synchronous and on_write_complete runs before any read can be processed, so this is unreachable there. This also explains the occasional test-cluster-disconnect.js timeout on Windows (build 93008), and it would have kept test-cluster-shared-leak.js rarely hanging on the round-robin path after fix 1.

Fix: on_ack_nack keeps a reply that arrives while the head of the queue is a handle message being written (early_ack_nack), and on_write_complete applies it once the message has moved into waiting_for_ack; a NACK stored this way goes through the normal retransmit path. The stash is taken on every write completion, and the queue head cannot change while a write is in progress (insert_message inserts behind it), so it can only ever apply to the message it was read for.

Tests (`test/js/node/cluster.test.ts`)

SCHED_NONE: a second worker listens on the same shared handle (existing, all platforms) now also counts the primary's own net.Server listen() calls, which only a RoundRobinHandle makes, and expects 1 on Windows and 0 elsewhere. This pins which handle a SCHED_NONE TCP listen gets on each platform; the rest of the SCHED_NONE suite passes identically either way (which is how the Windows runs below can pass at all). On the canary it fails on Windows with primary listen() calls: 0.
SCHED_NONE: plain worker listening on a key owned by a TLS shared-only handle (all platforms): the existing TLS-then-plain fixture run with NODE_CLUSTER_SCHED_POLICY=none. On POSIX the plain worker shares the TLS worker's socket (node's behaviour, unchanged); on Windows it gets EINVAL with the reworded hint. On the canary the Windows run reports the worker listening on the TLS socket instead.
SCHED_NONE / SCHED_RR: two workers on one port keep answering IPC while connections arrive, then exit (Windows-only, since both hangs are): 100 sequential connections to two workers on one port, pinging both workers over IPC after each one, then disconnect() and both exit 0. The SCHED_NONE variant fails 10/10 runs on the canary (worker N stopped answering after connection K, the blocked accept()); the SCHED_RR variant exercises the handoff ack path and, as described above, only failed on the unfixed build under load (it is what caught fix 2 in CI). Both take about 2.0s on a debug build, 1.3s of which is starting three debug processes.
The existing TLS-vs-round-robin EINVAL test gets the same 30s budget as its two siblings; it takes 4.9-5.0s on a Linux debug build (three debug process startups plus loading node:tls) and was failing on the default 5s timeout for me while the machine was busy. Unrelated to the fix otherwise.

Verification

Windows x64, debug build of this branch, vendored tests invoked the way the CI runner invokes them:

Round-robin fixture in 12 parallel copies x 100 connections: 0 stalls in 36 instances (unfixed debug build: 5 of 12 instances stalled, twice); same with the SCHED_NONE fixture, 0 in 24.
test-cluster-shared-leak.js: 40/40 pass (canary through the same loop: 3/40 time out).
All 85 test-cluster-*.js files plus the test-child-process-fork*/send*/pass-fd handle tests: 111/111 pass.
cluster.test.ts: 26 pass, 9 skipped (the pre-existing Windows skips); spawn.ipc*.test.ts, child_process_ipc_handle.test.ts, child_process.test.ts: 64 pass, 0 fail.
Two idle SCHED_NONE workers on one port: 0ms user / 0ms system each.

Linux x64, debug build: cluster.test.ts 32 pass / 3 skip (the Windows-only tests), the IPC test files above pass, and all 85 vendored test-cluster-* files pass.

On Windows, a listening socket duplicated into several workers cannot be accepted from by all of them: the worker that loses the race for a connection blocks inside accept() (mswsock checks for a pending connection, then waits for one), so its event loop never runs again, and libuv's exclusive AFD polls on the same socket from two processes cancel each other continuously. Create a RoundRobinHandle for TCP listens there regardless of the scheduling policy, and apply the TLS/non-TLS mixing check whenever a listen is round-robin.

…e comment

robobun · 2026-08-12T12:13:27Z

^{Updated 11:02 AM PT - Aug 12th, 2026}

❌ @robobun, your commit 3d330c1 has 2 failures in Build #93459 (All Failures):

test/bake/deinitialization.test.ts - pid 3948 segmentation fault at address 0x0 on 🪟 2019 x64
scripts/build/ci.ts - build failed on 🐧 x64 - build-bun
scripts/build/ci.ts - build failed on 🐧 aarch64 - build-bun
scripts/build/ci.ts - build failed on :freebsd: aarch64 - build-bun
scripts/build/ci.ts - build failed on 🐧 x64-android - build-bun
scripts/build/ci.ts - build failed on 🍎 aarch64 - build-bun

🧪 To try this PR locally:

bunx bun-pr 37815

That installs a local version of the PR into your bun-37815 executable, so you can run:

bun-37815 --bun

coderabbitai · 2026-08-12T12:17:04Z

Walkthrough

The cluster primary process centralizes round-robin handle selection for scheduling and socket conditions. Windows TCP behavior under SCHED_NONE now uses round-robin handles. Tests cover repeated connections, IPC responses, clean exits, and TLS/non-TLS conflicts.

Changes

Cluster handle selection

Layer / File(s)	Summary
Centralized handle selection `src/js/internal/cluster/primary.ts`	`usesRoundRobinHandle()` now controls shared-handle conflict checks and handle construction. Windows UDP sharing still returns `ENOTSUP`.
Windows SCHED_NONE stress coverage `test/js/node/cluster.test.ts`	A Windows-only stress test connects to two workers over 100 connections, checks IPC responses, detects stalls, and verifies clean exits.
TLS sharing conflict coverage `test/js/node/cluster.test.ts`	The TLS fixture is reused, worker cleanup is centralized, and a Windows-only `SCHED_NONE` case checks the `EINVAL` TLS/non-TLS sharing error.

Possibly related PRs

oven-sh/bun#31829: Introduced the cluster handle-selection logic refined by this change.

Suggested reviewers: cirospaciari, jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the Windows cluster fixes and the related shared-leak test timeout issue.
Description check	✅ Passed	The description explains the problems, fixes, tests, platform behavior, and verification results in sufficient detail.

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js/internal/cluster/primary.ts`:
- Around line 255-256: Update kSharedOnlyHint in
src/js/internal/cluster/primary.ts:255-256 to identify the
RoundRobinHandle/primary TCP connection distribution constraint rather than
claiming the conflict occurs under SCHED_RR, and include a concrete remedy.
Update the Windows SCHED_NONE assertion in test/js/node/cluster.test.ts:959-961
to verify the corrected condition text.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 26e84ae9-e16c-474e-b5ec-9e5ac993eb82

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and 4705cc6.

📒 Files selected for processing (2)

src/js/internal/cluster/primary.ts
test/js/node/cluster.test.ts

claude

Beyond the inline nit, I checked that on POSIX usesRoundRobinHandle reduces exactly to the old inline conditions (so non-Windows behavior is unchanged), that the dropped handle !== undefined clause in the sharedOnly guard is subsumed by handle instanceof SharedHandle, and that Windows pipe listens (addressType === -1) under SCHED_NONE still fall through to SharedHandle as before.

Extended reasoning...

Verified the refactor is behavior-preserving on POSIX by expanding both the old and new predicates; the only behavioral delta is Windows+SCHED_NONE+TCP now taking the round-robin path, which is the stated intent and is covered by the two new Windows-only tests. The inline nit about the hint text is cosmetic and doesn't affect the fix.

… completes With libuv's asynchronous pipe writes the peer's NODE_HANDLE ack can be read before the write callback moves the handle message into waiting_for_ack. on_ack_nack dropped it, the write callback then parked the message, and every later message to that peer queued up behind it. Remember the early reply and apply it when the write completes. Also reword the TLS/non-TLS sharing hint, which no longer depends on SCHED_RR, and run the worker-responsiveness test under SCHED_RR as well.

…ly ack

robobun · 2026-08-12T13:48:44Z

Update since the first round of review:

The kSharedOnlyHint wording no longer names SCHED_RR (48816f2); the Windows SCHED_NONE test asserts the new text.
The first CI run of this PR failed its own new test once on Windows 2019 x64 with the primary distributing connections (worker 2 stopped answering after connection 52). That turned out to be a second, independent bug in the round-robin handoff: on Windows the worker's NODE_HANDLE ack can be read before libuv runs the write callback for the handle message, SendQueue::on_ack_nack dropped it, and the message then sat in waiting_for_ack forever with every later message queued behind it. Reproduced on the canary by running the fixture in 10-12 parallel copies (4/10 release, 5/12 debug, with a probe confirming the state in every stalled instance); 0 stalls in 36 + 24 instances with the fix. Fixed in src/runtime/ipc.rs (48816f2), and the responsiveness test now also runs under SCHED_RR. The PR description has the full write-up of both problems.
The comment-cop flags on the remaining comments: shortened the three it flagged first (6071702); the two it still flags are a four-line note on the platform constraint behind the win32 branch and a two-line field doc, replied to inline.

claude

I reviewed this PR and didn't find any bugs; the earlier note on the kSharedOnlyHint wording is addressed. Because it changes Windows cluster scheduling semantics (TCP under SCHED_NONE silently routes through the primary) and adds ordering-sensitive state to the IPC send queue, a human look would still be worthwhile.

What was reviewed:

usesRoundRobinHandle reduces to the previous predicate on POSIX (SharedHandle guard and constructor selection unchanged for non-win32); pipes (addressType === -1) on Windows still take the SharedHandle path.
early_ack_nack is only stashed when write_in_progress && queue[0].handle.is_some() and is unconditionally .take()n in on_write_complete, so it cannot survive to a different message; replaying it under Done::AwaitAck reaches the normal ack path (waiting_for_ack is now populated) which ends in continue_send.
AckNack is a plain enum, so the new Cell<Option<AckNack>> needs no cleanup in Drop/close_socket.

Extended reasoning...

Overview

Three files touched. src/js/internal/cluster/primary.ts extracts usesRoundRobinHandle(message) and, on Windows, forces TCP listens (addressType 4/6) through RoundRobinHandle regardless of schedulingPolicy, working around the Winsock accept race documented in the description; the sharedOnly/RR mismatch guard and the handle-constructor selection now both use the shared predicate, and the kSharedOnlyHint text is reworded to no longer name SCHED_RR. src/runtime/ipc.rs adds an early_ack_nack stash on SendQueue so a NODE_HANDLE ack that arrives before libuv's write callback (Windows-only ordering) is applied once the message reaches waiting_for_ack instead of being dropped. test/js/node/cluster.test.ts adds three Windows-only tests, extracts the TLS-vs-net fixture into a shared constant with a hard failure on the previously-hanging path (netWorker.on('listening', ...)), and bumps one existing test's timeout to 30s to match its siblings.

Security risks

None identified. No untrusted-input parsing changes; the ipc.rs change only reorders when an already-received ack is applied.

Level of scrutiny

Moderate-to-high. The primary.ts refactor is behavior-preserving on POSIX (verified by expanding the predicate against each removed condition), but on Windows it makes a deliberate semantic choice — SCHED_NONE for TCP is silently upgraded to primary-distributed connections — which is a design decision a maintainer should ratify (the description argues it is unavoidable without an AcceptEx-based listener, and notes TLS shared handles remain exposed). The ipc.rs change is small but sits in the IPC send-queue state machine; the invariant it relies on (queue head cannot change while write_in_progress) is enforced by insert_message inserting at index 1 in that state, and I traced the replayed on_ack_nack through both Ack (complete + continue_send) and Nack (retransmit via insert_message) paths.

Other factors

Since my previous inline comment on this PR, the ipc.rs fix was added (48816f2) after the PR's own new test caught a second, independent hang in CI, and the comment-cop-flagged comments were shortened (6071702); all prior review threads are resolved. The PR description records manual verification on Windows x64 debug (0/60 stalls under load, 40/40 test-cluster-shared-leak.js, 111 vendored cluster/child_process tests) and Linux x64 debug. No CODEOWNERS file exists in this repo. The bug hunter found nothing this round.

robobun · 2026-08-12T17:14:11Z

Heads-up on the TLS half this PR leaves out: #37896 makes TLS workers on Windows stop setting sharedOnly, so the primary gives them a RoundRobinHandle like plain workers (tls.Server wraps the handed-off connections). The two compose, with two things to reconcile in whichever lands second:

usesRoundRobinHandle's sharedOnly clause is then never true on Windows (still needed on POSIX), nothing to change there.
The new "SCHED_NONE on Windows: plain worker listening on a key owned by a TLS shared-only handle fails with EINVAL" test no longer holds once cluster: hand TLS workers their connections on Windows instead of sharing the listening socket #37896 is in: there is no shared-only handle on Windows any more, the plain worker joins the TLS worker's round-robin handle instead (node does not refuse this either). cluster: hand TLS workers their connections on Windows instead of sharing the listening socket #37896 marks the two existing EINVAL tests POSIX-only for the same reason, and also adds the 30s budget to the first one, so expect a small textual conflict around those tests.

…platform The existing SCHED_NONE tests pass identically whether the primary hands out a shared socket or distributes connections itself, so they could not tell the two apart. Count the primary's own net.Server listen() calls (only RoundRobinHandle makes any) and run the TLS-then-plain fixture under SCHED_NONE on every platform: the plain worker shares on POSIX and gets EINVAL on Windows.

robobun · 2026-08-12T17:37:15Z

Self-review flagged that nothing pinned the POSIX side of the new platform branch (every existing SCHED_NONE test passes identically with either handle, which is exactly what the Windows runs show). ad8a17d adds two pins: the existing two-workers-one-port test now counts the primary's own net.Server listen() calls (1 on Windows, 0 elsewhere, 0 on the canary's Windows), and the TLS-then-plain fixture also runs under SCHED_NONE on every platform (shares on POSIX, EINVAL on Windows). Verified on the Windows box and on Linux; the PR description's test section is updated.

robobun · 2026-08-12T17:49:18Z

Status: ready for review. The source here is identical to what build 93234 ran green across all lanes (cluster tests included); the two builds since (93440 for the test-only pin commit and the 93459 retry) failed in the build-bun steps only, because the agents could not download c-ares, mimalloc, lol-html and WebKit from GitHub ("Failed to download after 5 attempts"), which is hitting every build in the queue right now. The test-pin commit was verified locally on Windows x64 and Linux x64 (both pins fail on the canary, pass with this branch). Needs a CI retry once the downloads work again, or a maintainer merge.

claude

I reviewed this PR and didn't find any bugs (my earlier note on the kSharedOnlyHint wording was addressed in 48816f2). Because it changes the IPC send-queue state machine and overrides SCHED_NONE on Windows for TCP, a human look would still be worthwhile.

What was reviewed:

usesRoundRobinHandle: on POSIX reduces to the previous schedulingPolicy === SCHED_RR with the same UDP/sharedOnly exclusions; only Windows TCP behavior changes.
early_ack_nack in SendQueue: stashed only when write_in_progress && queue[0].handle.is_some(), .take()n unconditionally in on_write_complete, applied only in the Done::AwaitAck arm — so it cannot leak across messages or apply to a non-handle write.
insert_message inserts at index 1 when write_in_progress, so the queue head cannot change under the in-flight write the stash was recorded for.
New Windows responsiveness tests use bounded polling (10s per-ping timeout inside a 30s test budget), not sleeps.

Extended reasoning...

Overview

This PR fixes two independent Windows-only hangs in node:cluster that leave a worker permanently unresponsive to disconnect(), causing test-cluster-shared-leak.js timeouts in CI since #31829:

src/js/internal/cluster/primary.ts: factors handle-selection into usesRoundRobinHandle(message) and forces Windows TCP listens to use RoundRobinHandle regardless of schedulingPolicy, because two processes accepting on WSADuplicateSocketW copies of one listener race such that the loser blocks inside accept() (and idle workers spin on mutually-cancelling exclusive AFD polls). The shared-only/round-robin conflict guard now uses the same predicate, and the hint text drops the "under SCHED_RR" phrasing.
src/runtime/ipc.rs: adds early_ack_nack: Cell<Option<AckNack>> to SendQueue. On Windows, a worker's NODE_HANDLE ack can be read before libuv fires the write callback for the handle message; previously on_ack_nack dropped it (nothing in waiting_for_ack yet) and the message then sat in waiting_for_ack forever. Now the ack is stashed and applied when on_write_complete moves the message into waiting_for_ack.
test/js/node/cluster.test.ts: adds a Windows-only 100-connection responsiveness test under both policies, pins which handle type each platform uses under SCHED_NONE (via a net.Server.prototype.listen counter), adds a SCHED_NONE variant of the TLS-then-plain fixture with per-platform expectations, and gives the existing TLS-vs-round-robin test a 30s budget.

Security risks

None identified. No user-controlled input parsing, no auth/crypto/permissions code. The changes are internal IPC sequencing and cluster handle routing.

Level of scrutiny

High. Both fixes touch concurrency-sensitive paths:

The early_ack_nack change alters the SendQueue state machine that gates every handle-carrying IPC message. I traced the invariant: the stash is set only when write_in_progress and the queue head has a handle; on_write_complete unconditionally .take()s it and applies it only in Done::AwaitAck (which itself requires the head had a handle), so it cannot survive to a later message. insert_message inserts behind index 0 when a write is in progress, so the head cannot change out from under the stash. On POSIX the write completes synchronously before any read, so the stash path is unreachable there — but the code is not cfg-gated, which is fine since it's a no-op.
Forcing RoundRobinHandle for Windows TCP under SCHED_NONE is a user-visible semantic change: cluster.schedulingPolicy still reports SCHED_NONE but connections are distributed by the primary. This is arguably the right call given the alternative is a hung event loop, and the PR notes the real fix (AcceptEx-based listeners) is out of scope. A maintainer should confirm this tradeoff and the interaction with #37896 (which the author flagged will require reconciling the new SCHED_NONE-on-Windows EINVAL test).

Other factors

The PR description contains unusually thorough root-cause analysis (native stack dumps, Winsock experiments, load-based reproduction rates) and the author verified 40/40 passes of the previously-flaky vendored test plus 111 related test-cluster-*/test-child-process-* files on Windows.
My prior inline comment (hint text naming SCHED_RR) and CodeRabbit's identical finding were both addressed. The comment-cop bot's remaining flags were argued in-thread and the retained comments (4-line platform-constraint note, 2-line field doc) look justified per REVIEW.md's "only comment what the code cannot say."
Current CI (build 93459) is failing on infrastructure (dependency downloads), not this code; build 93234 with identical source was green across all lanes per the author's status comment.

robobun · 2026-08-12T18:15:38Z

Build 93459 (the retry, same content as ad8a17d) has since finished: every lane whose build step got past the GitHub download failures ran the full suite, and that includes all 16 Windows test jobs (2019 x64 and 11 aarch64), alpine x64/aarch64 and debian x64-asan. No cluster failures on any of them, so the new race tests and both pins pass on Windows CI and the cross-platform pins pass on Linux; the only red there is test/bake/deinitialization.test.ts segfaulting on Windows 2019, which is failing on main independently of this PR (reported separately). Build 93234 earlier ran the same source green on all lanes. The debian/ubuntu/darwin test lanes in 93459 never ran only because their build steps could not download dependencies.

Re the #37896 heads-up: agreed the two compose. Note that since ad8a17d the SCHED_NONE TLS-then-plain test here is cross-platform and branches on isWindows (shares on POSIX, EINVAL on Windows), so once TLS workers get a RoundRobinHandle on Windows as well, its Windows branch collapses into the POSIX expectation; whichever of the two lands second can fold that in.

…mpty stdout as null The four tests whose completion depends on IPC after a round-robin handoff run one at a time on Windows. A busy machine can make the primary drop a worker's ack there (#37815), and every later message to that worker waits behind it. A fixture that dies before it prints leaves stdout empty. Parse that as null so the failure shows stderr instead of a JSON SyntaxError.

On Windows uv_write completes later, so the peer's ack or nack for a handle message can be read before the write callback moves that message into `waiting_for_ack`. on_ack_nack dropped such a reply. The message then waited for an ack that never came, and every later message on the channel queued behind it. A postponed disconnect() waits for that ack, and on Windows a handle sent in the same tick as disconnect() is always still queued. The reply is now kept in the Windows write state and applied when the write completes. A kept nack takes the normal retransmit path. The queue head cannot change while a write is in progress, so the reply can only apply to the message it was read for. This is the ipc.rs change of #37815, kept behind cfg(windows): a POSIX write completes inside write(), so no reply can be read in between.

github-actions · 2026-11-11T00:44:16Z

Closing this PR because it has been inactive for more than 90 days.

robobun · 2026-11-11T00:55:12Z

This PR stays closed. Two open PRs now cover its two parts.

The accept() race on a shared listening socket: usockets: accept with AcceptEx on a listening socket that another process holds (Windows) #41358 accepts with AcceptEx on such a socket, and net, tls: consolidate the open node:net, node:tls, Bun.connect, Bun.listen and usockets pull requests #46522 includes that change. The socket stays shared, as in Node. The change also covers child.send(msg, server) and TLS workers. The usesRoundRobinHandle routing from this PR is not necessary with it.
The early handle ACK in SendQueue: ipc: postpone disconnect() while a handle send is still queued #43081 carries the ipc.rs change from this PR.

main at 89452d2 still has both causes. test/flaky-tests.txt lists test-cluster-shared-leak.js for these timeouts (8 of 80 builds, Windows).

robobun added 2 commits August 12, 2026 10:56

cluster: probe the Windows accept race with single connections, refin…

4705cc6

…e comment

robobun assigned cirospaciari Aug 12, 2026

github-actions Bot added the claude label Aug 12, 2026

robobun mentioned this pull request Aug 12, 2026

cluster: port Node's cluster and child_process handle-passing suites (+43 upstream tests; cluster 54 → 85) and implement what they expose — round-robin fd handoff, SCHED_NONE shared handles, UDP clustering, IPC handle passing #31829

Merged

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/js/internal/cluster/primary.ts Resolved

claude Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/js/internal/cluster/primary.ts Resolved