http: closeAllConnections() must not stop the listener

[robobun]

Fixes #31301 (reported by @mxschmitt, who also has a fix in flight as #31302 — see the comparison at the bottom).

Repro

const http = require("node:http");
const srv = http.createServer((req, res) => res.end("ok"));
srv.on("close", () => console.log("EVENT: close fired"));
srv.listen(0, "127.0.0.1", async () => {
  const port = srv.address().port;
  srv.closeAllConnections();
  console.log("listening:", srv.listening);
  try {
    console.log("status:", (await fetch(`http://127.0.0.1:${port}/`)).status);
  } catch (e) {
    console.log("ERROR:", e.code);
  }
  srv.close(err => console.log("close(cb) err:", err?.code ?? null));
});

	node	bun (before)	bun (after)
listening	true	false	true
new request	200	ECONNREFUSED	200
'close' event	not fired	fired	not fired
close(cb)	null	ERR_SERVER_NOT_RUNNING	null

Cause

Server.prototype.closeAllConnections() nulled this[serverSymbol] and called the native stop(true), which is a full shutdown of the listen socket and every connection:

Server.prototype.closeAllConnections = function () {
  const server = this[serverSymbol];
  if (!server) return;
  this[serverSymbol] = undefined;
  clearInterval(this[kConnectionsCheckingInterval]);
  this.listening = false;
  server.stop(true);
};

Node's contract is narrower: destroy the connections, keep accepting. The API exists for a graceful reload/drain, so on Bun the drain step was an outage.

Two consequences beyond the listener dying:

• listening, the 'close' event and close(cb) all reported a server that was never asked to stop.
• Because close() also clears this[serverSymbol], the standard close(); closeAllConnections() teardown was a no-op for the forced half. The @azure/msal-node loopback client uses exactly that sequence and hangs on Bun (getTokenInteractive in @azure/msal-node will cause Bun to hang after the end of the script #30501).

Fix

Iterate kTrackedConnections (the set the 'connection' event and getConnections() already maintain) and destroy() each socket, exactly like Node. Nothing else is touched, which is also what makes it work after close().

Server.prototype.closeAllConnections = function () {
  const connections = this[kTrackedConnections];
  if (!connections) return;
  for (const socket of connections) {
    socket.destroy();
  }
};

Destroying through the JS socket (rather than closing the underlying uSocket) is what gives the socket object Node's observable state: socket.destroyed === true and a 'close' event. The server.on("connection", s => set.add(s)) + s.on("close", () => set.delete(s)) idiom depends on it.

Verification

test/js/node/http/node-http-close-all-connections.test.ts (new, 4 tests, all pass unmodified on Node.js):

• connections destroyed (destroyed flag + 'close' event), listener still accepting, no server 'close' event, close(cb) reports no error
• close() then closeAllConnections() destroys in-flight sockets so the close callback runs
• every tracked connection is destroyed (4 sockets)
• no-op on a server that never listened

3 of the 4 fail on main. test/js/node/test/parallel/test-http-server-close-all.js and the other 356 test-http-* node parallel tests are unaffected.

Five existing tests used closeAllConnections() as a stand-in for close(), and two asserted the old behaviour directly (listening === false, the connections-checking interval destroyed). Both of those now assert what Node does: closeAllConnections() leaves them alone, close() changes them.

Known limitation

A TCP connection that has been accepted but has not yet sent a request head has no JS socket wrapper, so it is not in kTrackedConnections and survives closeAllConnections(). Node tracks connections from TCP-accept and would destroy it.

This is the same architectural gap already noted on getConnections() ("the native server does not surface raw accepts to JS yet") — 'connection' does not fire for those sockets either, so they are invisible to the whole connection-tracking surface, not just this method. They are still reaped by headersTimeout/requestTimeout. Closing it properly needs a native "close every socket, keep the listener" primitive, which is what #31302 adds.

Relationship to #30505 and #31302

Both are open and neither is a duplicate of the other:

• fix(node:http): keep listener open in Server.closeAllConnections #31302 (@mxschmitt) fixes this same listener-teardown bug by adding a native closeAllConnections primitive through uWS → libuwsockets.cpp → App.rs → host fn → class registration, and delegating to it. That catches the pre-request sockets above, which this PR does not. But routing only through the native layer leaves the JS socket wrapper stale: bun's existing closeIdleConnections() already takes that path, and on it socket.destroyed stays false and the socket never emits 'close', where Node gives true / fires. It also still early-returns after close(), so it does not address getTokenInteractive in @azure/msal-node will cause Bun to hang after the end of the script #30501.
• fix(http): preserve server reference across close() for closeAllConnections() #30505 fixes the getTokenInteractive in @azure/msal-node will cause Bun to hang after the end of the script #30501 hang by keeping this[serverSymbol] alive past close() and teaching the Rust stop_listening/stop_from_js to force-close the app when the listener is already gone, so closeAllConnections() can still reach stop(true). It keeps the full-stop, so the listener-teardown bug remains.

This PR removes the stop(true) call instead, which fixes both bugs in 11 lines of JS with no Rust change: the #30501 teardown sequence exits normally on this branch (verified against Node — both exit, main hangs).

The complete fix is probably this PR's socket.destroy() loop plus #31302's native sweep for the pre-request sockets. Happy to fold that in with credit, or to close this in favour of #31302 plus a small JS change there — whichever reviewers prefer.

[robobun]

^{Updated 7:09 PM PT - Jul 5th, 2026}

✅ @robobun, your commit 0175067df671bb4067c9564554f6ad6de73e54a8 passed in Build #68678! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 33394

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

bun-33394 --bun

[github-actions]

Found 1 issue this PR may fix:

1. node:http: Server.closeAllConnections() shuts down the listening socket #31301 - Reports that closeAllConnections() shuts down the listening socket instead of only destroying active connections, which is exactly what this PR fixes

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #31301

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 13 minutes

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: dbcbca5b-32db-4370-b812-8c45ed77a0a7

📥 Commits

Reviewing files that changed from the base of the PR and between 93a8760 and 0175067.

📒 Files selected for processing (9)

• src/js/node/_http_server.ts
• test/js/bun/test/parallel/test-http-server.listening-should-work.ts
• test/js/bun/test/parallel/test-http-timeout-destruction-should-be-visible-using-kConnectionsCheckingInterval.ts
• test/js/first_party/ws/ws.test.ts
• test/js/node/http/node-http-close-all-connections.test.ts
• test/js/node/http/node-http-with-ws.test.ts
• test/js/node/http/node-http.test.ts
• test/js/web/fetch/client-fetch.test.ts
• test/js/web/fetch/fetch.stream.test.ts

Walkthrough

Server.prototype.closeAllConnections now destroys tracked sockets without stopping the HTTP server. Tests were updated to keep server.listening true after that call, and several teardown paths now call server.close() to finish shutdown.

Changes

closeAllConnections Behavior Update

Layer / File(s)	Summary
Core implementation change src/js/node/_http_server.ts	closeAllConnections now destroys sockets from the tracked-connections set instead of clearing the checking interval, unmarking listening, and calling server.stop(true).
New dedicated test suite test/js/node/http/node-http-close-all-connections.test.ts	Adds a shared listenAndConnect() helper and tests covering destroying active connections while the server keeps listening, in-flight request handling, multiple tracked connections, and calling the method before listening starts.
Existing test and caller updates test/js/bun/test/parallel/test-http-server.listening-should-work.ts, test/js/bun/test/parallel/test-http-timeout-destruction-should-be-visible-using-kConnectionsCheckingInterval.ts, test/js/first_party/ws/ws.test.ts, test/js/node/http/node-http.test.ts, test/js/web/fetch/client-fetch.test.ts, test/js/web/fetch/fetch.stream.test.ts, test/js/node/http/node-http-with-ws.test.ts	Updates assertions to expect listening stays true and the interval is not destroyed after closeAllConnections(), and adds explicit server.close() calls, with "close" event waits where needed, in cleanup and teardown paths.

Related PRs: None identified.

Suggested labels: bun:http, tests

Suggested reviewers: None identified.

🐰 A rabbit hops through sockets tight,
Closing connections, keeping the light—
The server still listens, still stands its ground,
Till .close() is called, then silence found.
Tests now confirm this gentler way,
Connections destroyed, but the door stays at bay!

🚥 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 is concise and accurately states the main behavior change: closeAllConnections should not stop the listener.
Description check	✅ Passed	The description clearly explains the fix and includes verification details, though it does not use the template's exact headings.

---

_{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 `@test/js/node/http/node-http-close-all-connections.test.ts`:
- Around line 1-115: Move these `closeAllConnections()` tests into the existing
`node-http.test.ts` file instead of keeping them in a new
`node-http-close-all-connections.test.ts` file. Keep the same coverage and
assertions, but place the new cases alongside the other `node:http` tests so the
existing `listenAndConnect`-style setup and related `createServer` coverage stay
consolidated in one test module.

🪄 Autofix (Beta)

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: 28166af6-4367-41ba-b84e-7d4e59b54eaa

📥 Commits

Reviewing files that changed from the base of the PR and between d37f520 and 2a9f783.

📒 Files selected for processing (8)

• src/js/node/_http_server.ts
• test/js/bun/test/parallel/test-http-server.listening-should-work.ts
• test/js/bun/test/parallel/test-http-timeout-destruction-should-be-visible-using-kConnectionsCheckingInterval.ts
• test/js/first_party/ws/ws.test.ts
• test/js/node/http/node-http-close-all-connections.test.ts
• test/js/node/http/node-http.test.ts
• test/js/web/fetch/client-fetch.test.ts
• test/js/web/fetch/fetch.stream.test.ts

[github-actions]

This PR may be a duplicate of:

1. fix(http): preserve server reference across close() for closeAllConnections() #30505 - Also fixes closeAllConnections() to preserve the server reference across close(), addressing the same issue getTokenInteractive in @azure/msal-node will cause Bun to hang after the end of the script #30501
2. fix(node:http): keep listener open in Server.closeAllConnections #31302 - Also fixes closeAllConnections() to keep the listener open instead of shutting down the server

🤖 Generated with Claude Code

[robobun]

Thanks — addressing the four bot threads, two of which were real finds.

Missed sibling (node-http-with-ws.test.ts:96) — correct, fixed in 93a8760. It was the last bare closeAllConnections() teardown; node-http-res-settimeout-unref.test.ts, fetch-backpressure.test.ts and symlink-path-traversal.test.ts already pair it with close(), and test-http-10177-… calls process.exit(0).

Pre-request sockets — also correct, and I've written it into the PR body as a known limitation rather than pretending it isn't there. A socket that has been accepted but hasn't sent a request head has no JS wrapper, so it isn't in kTrackedConnections and survives. That gap already applies to 'connection' and getConnections() (the native server doesn't surface raw accepts to JS), and closing it needs a native "close every socket, keep the listener" primitive — which is exactly what #31302 adds. See below.

Fixes #31301 — added, with credit: @mxschmitt filed that issue and has a fix in flight as #31302.

Test file location — keeping the dedicated file for now. node-http.test.ts is ~3,700 lines and test/js/node/http/ already groups by topic (node-http-connect.test.ts, node-http-transfer-encoding.test.ts, node-http-maxHeaderSize.test.ts, …). It also carries request via http proxy, issue#4295, whose fixture binds localhost and then connects to localhost; on a host where those resolve to different families the test fails with ECONNREFUSED (plain Node fails the same way, so it isn't a Bun bug — just a non-hermetic fixture). That makes it an unreliable proof-of-fix target. Happy to move the tests if a maintainer would rather have them there.

---

On the overlap with #31302 and #30505

The duplicate-detector is right that all three touch closeAllConnections(), but they fix different things and none subsumes the others cleanly. The interesting part is a measurable difference between the two candidate fixes for this bug.

Destroying at the native layer leaves the JS socket object stale. closeIdleConnections() already takes the native path today, so it answers the question for #31302's approach without needing that branch:

const serverSocket = /* from server.on("connection") */;
server.closeIdleConnections();           // native us_socket_close
// node: socket.destroyed === true,  'close' fired
// bun:  socket.destroyed === false, 'close' never fires

Node's closeAllConnections() calls socket.destroy() on each tracked socket, so the socket object ends up destroyed and emits 'close'. Anything doing server.on("connection", s => set.add(s)) + s.on("close", () => set.delete(s)) depends on that — including Playwright's own TestServer, which is the code #31301 was filed about. Routing only through uWS gives a closed TCP connection but a socket object that still claims to be alive.

So, concretely:

	#31302 (native)	this PR (JS socket.destroy())
listener survives	✅	✅
socket.destroyed / 'close'	❌ stale	✅ matches Node
works after close() (#30501)	❌ early-returns	✅
pre-request sockets	✅ closed	❌ survive
diff	6 files, C++/Rust/TS	11 lines of JS

The complete fix is the union: this PR's socket.destroy() loop over the tracked sockets, then #31302's native sweep for the ones JS never saw. I'd rather not fold someone else's plumbing in uninvited, so: @mxschmitt, happy to hand you this JS half for #31302 (it's one commit), or to pull your native primitive into this branch with credit — your call, and apologies for the parallel PR, I only found yours after opening this one. Maintainers, if you'd rather just land the small one now and leave the pre-request sockets to a follow-up, this is ready.

#30505 is a separate question: it fixes the #30501 hang by keeping serverSymbol alive past close() so closeAllConnections() can still reach stop(true). This PR fixes that hang as a side effect of not calling stop(true) at all, with no Rust change, so if this lands #30505 can close.

[robobun]

Status: ready for review, the red check is CI infra

Every test lane that ran on build 68678 passed (284 jobs). The one red check is darwin-14-aarch64-test-bun, which Buildkite marked Expired — an agent never picked the job up, so no test ever ran on it. Nothing to fix from this side.

For the record, the previous build (68669) had three reds, none of which survived a rebase onto current main:

• test/bake/deinitialization.test.ts — ASAN heap-use-after-free during dev-server deinit (WRITE of size 1 ... thread T5) on x64-asan. All 20 x64-asan shards are green on the rebase. This diff has no path to it: src/bake/ contains no reference to node:http, _http_server, or closeAllConnections, and the fixture drives Bun.serve + the native server.stop(). Locally the file passes 20/20 under bun bd (debug + ASAN).
• test/js/node/net/net-mongodb-pattern-leak.test.ts on ubuntu-25.04-aarch64 — green on the rebase; this diff does not touch node:net, and it passes 3/3 locally.
• darwin-26-aarch64 — buildkite-agent artifact download timed out after 120s, no tests ran. Green on the rebase.

One note worth passing on, unrelated to this PR: the ASAN report in 68669 came through with no symbolized frames (the SIGABRT truncates it), so the bake deinit UAF it caught is invisible in the log beyond the header. A 1-byte write from a worker thread into freed memory during DevServer deinit looks like a real latent race rather than a one-off, and that area already carries several "UAF in deinit" fixes (#29949, #29988, #29951). Might be worth a look by someone who owns bake.

---

The fix itself is unchanged and green: closeAllConnections() destroys the tracked sockets and leaves the listen socket alone, 4 new tests (3 of which fail on main) that also pass unmodified on Node.js. The open question is still which of #33394 / #31302 / #30505 you want — see my comment above for the comparison, and #31302 for the offer to combine.

[robobun]

Closing in favour of #35839, which takes the same approach (destroy the sockets in kTrackedConnections, leave the listener alone) and extends it to closeIdleConnections(), with the upgraded-socket and partial-request-head guards that the connection tracking added in #32488 now makes necessary. The multi-connection test case from this PR has been folded into #35839.

Verified against a fresh main build first: this scenario is not fixed on main yet, so the fix continues in #35839.