node:cluster: worker.disconnect() drops queued send() messages and the worker never sees 'disconnect'

[robobun]

worker.send() followed by worker.disconnect() is the documented graceful-shutdown pattern for cluster workers. In Bun the primary discards every message still queued in the IPC write buffer and the worker never gets its 'disconnect' event, so rolling-restart / drain loops silently lose work.

Reproduction

import cluster from "node:cluster";
if (cluster.isPrimary) {
  const w = cluster.fork();
  await new Promise(r => w.once("message", r));
  const payload = Buffer.alloc(64 * 1024, "z").toString();
  for (let i = 0; i < 50; i++) w.send({ i, payload });   // every send() returns true
  w.disconnect();
  // node: worker receives all 50, then 'disconnect', then exits
  // bun:  worker receives ~3, sees the channel abruptly close, exits
} else {
  let n = 0;
  process.on("message", () => n++);
  process.on("disconnect", () => { console.log("got", n); process.exit(0); });
  process.send("ready");
}

Node v26: got 50. Bun 1.4.0: got 3 (varies with scheduling) and the worker's 'disconnect' listener never fires.

Cause

Worker.prototype.disconnect in internal/cluster/primary.ts calls this.process.disconnect() right after queueing the internal {act:'disconnect'} message. Node's primary.js has never had this call: the primary only enqueues {act:'disconnect'} (ordered after the user's messages on the same channel) and the worker closes the channel from its side once it has drained its servers. Closing the primary's end first discards the still-queued outbound writes, including the internal disconnect message itself, so the worker treats it as an unexpected channel close and exits before user 'disconnect' listeners run.

Removing that call exposes a second bug in the worker-side _disconnect: internal/cluster/child.ts and node:net each declared their own private Symbol("owner_symbol"), so handle[owner_symbol] on a cluster faux handle was always undefined and _disconnect fell through to the faux close() (which ignores its callback). waitingCount never reached zero and the worker never called process.disconnect(). Previously the primary's force-close masked this by closing the channel anyway; without it, a worker holding a net.Server would hang forever on disconnect().

Fix

• internal/cluster/primary.ts: drop the this.process.disconnect() call so the worker owns channel teardown, matching Node.
• internal/shared.ts: a single shared owner_symbol that node:net and internal/cluster/child both import.
• node:net listenInCluster: set handle[owner_symbol] = server on the cluster faux handle and close it when the server closes, so _disconnect can call server.close(cb) and complete.

This is a subset of the src/ changes in #30548, which reached the same primary-side conclusion while fixing the server-shutdown hang. That PR additionally handles http.Server in cluster workers via a new _trackServer hook; this PR is narrower so the data-loss fix can land independently.

Verification

New test in test/js/node/cluster.test.ts sends 50 messages with 64 KiB payloads (enough to back up past the kernel socket buffer), calls worker.disconnect(), and asserts the worker received all 50 and observed 'disconnect'. On current main: received: 3, workerSawDisconnect: false. With this change it matches Node. test-cluster-worker-wait-server-close.js's in-worker assertion (serverClosed before 'disconnect') now passes instead of firing in a worker that was already being force-exited. All 54 test/js/node/test/parallel/test-cluster-*.js tests pass.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3473be75-d046-4c7c-9f24-3b1323aee10f

📥 Commits

Reviewing files that changed from the base of the PR and between 3316c48 and a2eba0c.

📒 Files selected for processing (5)

• src/js/internal/cluster/child.ts
• src/js/internal/cluster/primary.ts
• src/js/internal/shared.ts
• src/js/node/net.ts
• test/js/node/cluster.test.ts

---

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

[robobun]

^{Updated 2:05 AM PT - Jul 7th, 2026}

❌ @robobun, your commit a2eba0c has 1 failures in Build #69627 (All Failures):

• test/napi/napi.test.ts - code 1 on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 33594

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

bun-33594 --bun

[github-actions]

This PR may be a duplicate of:

1. node:cluster: let workers shut down gracefully on worker.disconnect() #30548 - Superset of this PR's src/ changes; both remove this.process.disconnect() from Worker.prototype.disconnect, export a shared owner_symbol from internal/shared, and wire it into listenInCluster in node:net

🤖 Generated with Claude Code

[robobun]

Correct, this is intentionally a subset of #30548's src/ changes (see the Fix section of the PR body). #30548 has been open since May with a broader scope (it also adds a cluster._trackServer hook for http.Server in workers); this PR carries only the data-loss fix plus the minimum worker-side change to keep test-cluster-worker-wait-server-close.js from hanging once the primary stops force-closing the channel. It adds a message-delivery regression test that #30548 does not have. Either PR landing resolves the message drop; this one is the smaller review surface.

[claude]

I didn't find any bugs, but this changes the cluster disconnect protocol (shifting channel-teardown ownership from primary to worker) and overlaps with #30548, so a maintainer should sign off on the lifecycle change and decide how to reconcile the two PRs.

Extended reasoning...

Overview

This PR fixes worker.disconnect() in node:cluster dropping queued IPC messages. It removes this.process.disconnect() from the primary's Worker.prototype.disconnect (so the worker owns channel teardown, matching Node), unifies the previously-duplicated owner_symbol between node:net and internal/cluster/child via internal/shared, and wires the cluster faux handle to its owning net.Server in listenInCluster so the worker-side _disconnect can complete. A new regression test in test/js/node/cluster.test.ts sends 50×64KiB messages before disconnect() and asserts full delivery plus 'disconnect' observation on both sides.

Security risks

None identified. This is purely IPC lifecycle/ordering within the cluster module; no auth, crypto, or untrusted-input parsing is touched.

Level of scrutiny

Moderate-to-high. Although the diff is small (~25 lines of src/ changes), it inverts who closes the IPC channel during graceful shutdown. The PR body itself notes that removing the primary's force-close exposed a latent worker-side hang that had to be fixed in the same PR — which means correctness now depends on the worker always reaching process.disconnect() via the owner_symbol/server.close(cb) path. If any server type (the PR body mentions http.Server is handled only in #30548) or edge case leaves waitingCount stuck, the worker hangs where it previously would have been force-closed. That's a real behavioral trade-off a maintainer should weigh.

Other factors

• There's an open, overlapping PR (#30548) that is a superset of these src/ changes; a human should decide whether to land this narrower fix first or fold it into the broader one.
• The listenInCluster change adds server.once("close", () => handle.close()) — a new lifecycle coupling in node:net that interacts with the faux-handle close() and the handles map in child.ts. It looks correct (close() guards on key === undefined), but it's the kind of subtle ordering change that benefits from a reviewer familiar with the cluster round-robin implementation.
• The PR reports all 54 test-cluster-* parallel tests pass and adds a solid regression test, which is reassuring.

[robobun]

Re the review's concern that an http.Server worker might now hang where it was previously force-closed: verified empirically that this is already the behavior on main, so there's no regression here.

// worker: http.createServer(...).listen(0)
// primary: wait for 'listening', then worker.disconnect()

	events on primary
node v26	listening, disconnect, exit:0
bun main	listening, disconnect (worker never exits)
this PR	listening, disconnect (worker never exits)

Bun's _http_server.ts binds its own SO_REUSEPORT socket instead of going through cluster._getServer, so the server is never in the worker's handles map and _disconnect's loop is empty regardless of this change: waitingCount reaches 0 immediately and the channel closes, but the listening socket keeps the worker alive. On main the primary's force-close didn't change that. #30548's cluster._trackServer hook is what registers the http server so _disconnect can close it; that remains the right place to fix it.

[robobun]

Status: ready for review. No cluster, net, or IPC test has failed on either CI run of this branch.

Remaining red on build 69627 is unrelated to the diff:

• :darwin: 26 aarch64: buildkite-agent artifact download timed out after 120s before any tests ran (same on the previous build)
• :windows: 2019 x64: test/napi/napi.test.ts "Condition was not met after 100 GC attempts" (N-API GC flake, also present on the previous build)
• flaky annotation: spawn.test.ts timeout and net-mongodb-pattern-leak.test.ts RSS 9.4 MB vs 8 MB bound, both passed on retry; the leak test doesn't touch cluster and the only net.ts change here is inside listenInCluster's worker-only callback

The new test (worker.disconnect() delivers queued send() messages) and all 54 test-cluster-* parallel tests pass on every lane that actually ran.