fix(net): destroy socket on native close to prevent server.close() hang

[bilby91]

Summary

• When the native Bun socket closes, the close handlers (SocketHandlers, SocketHandlers2, ServerHandlers) push null to the readable stream but never call destroy(). If the readable is paused or the writable was never ended, autoDestroy never fires, leaving the socket in a zombie state (_handle=null, destroyed=false). This causes server._connections to never decrement, so server.close() hangs forever.
• Added process.nextTick(destroyNT, self) in all three close handlers to match Node.js behavior where the native handle's close callback destroys the socket directly.
• Added regression tests covering paused sockets, unpiped transforms, and pipe+pause+end sequences.

Fixes #13184, fixes #19563, fixes #23648

Test plan

• New regression test test/regression/issue/13184.test.ts with 6 test cases:
  • Paused socket gets destroyed on native close
  • Socket with end() called and paused readable gets destroyed
  • Unpiped socket gets destroyed on native close
  • Pipe + pause + end sequence gets destroyed
  • Socket that was never read gets destroyed on native close
  • destroyed flag is true after native close
• All tests fail with USE_SYSTEM_BUN=1 (unpatched) and pass with debug build
• All existing test/js/node/net/ tests pass (155 pass, pre-existing failures unchanged)
• All test/js/bun/http/serve.test.ts tests pass (189 pass)
• All scenarios verified against Node.js v22.12.0 (all pass)

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 72b43592-0e08-4533-a510-68aca0e5664e

📥 Commits

Reviewing files that changed from the base of the PR and between da62a73 and f600603.

📒 Files selected for processing (1)

• test/regression/issue/13184.test.ts

---

Walkthrough

Schedules a next-tick call to destroyNT(...) for undestroyed sockets in multiple net handlers so they reliably enter the destroyed state and emit close; adds a regression test verifying server.close() completes across various client teardown and stream-state scenarios.

Changes

Cohort / File(s)	Summary
Net handler updates src/js/node/net.ts	Added next-tick destroyNT(self)/destroyNT(data) calls in SocketHandlers.close, ServerHandlers.close, and _handle-based SocketHandlers2.close for sockets that are not destroyed; removed an obsolete TODO comment.
Regression tests test/regression/issue/13184.test.ts	Added tests exercising net.Server.close() completion when accepted sockets close via destroy() or end() across paused/readable/piped/unpiped stream states and asserting server-side socket close/destroyed behavior.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing socket destruction on native close to prevent server.close() hanging, which directly addresses the core issue.
Description check	✅ Passed	The description comprehensively covers the problem, solution, test plan with 6 test cases, and test results against Node.js v22.12.0 and existing tests.
Linked Issues check	✅ Passed	Code changes directly address all three linked issues by implementing socket destruction on native close, preventing zombie sockets and allowing server.close() and process exit to work correctly.
Out of Scope Changes check	✅ Passed	All changes are in scope: native socket close handlers modified to call destroyNT(), and comprehensive regression tests added to prevent future regressions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/13184.test.ts`:
- Around line 31-45: The test is using fixed setTimeout delays (the
client.destroy() delay, the 500ms propagation delay, and the 5s hang detector)
which makes it flaky; replace these sleeps with event-driven waits: after
triggering client.destroy() await the client's 'close'/'end'/'destroy' event (or
the socket 'close' event) instead of waiting 100ms, replace the 500ms
propagation sleep with awaiting the server-side socket 'close' or connection
'end' event, and replace the 5s reject timeout around server.close() with
resolving/rejecting based on the server.close callback or the server's 'close'
event so server.close() completion is awaited directly (refer to
client.destroy(), server.close(), and the Promise wrappers in this test to find
where to swap timeouts for event listeners).
- Around line 31-34: Add a parallel test variant that exercises a graceful
client close (client.end()) instead of the current abrupt teardown
(client.destroy()): duplicate the existing teardown block that currently calls
client.destroy() inside the setTimeout and replace it with client.end(),
ensuring the test still resolves after the timeout so the server-side cleanup
path for a FIN (half-close) is exercised; update any assertions or awaits that
expect socket state transitions if necessary to account for the half-close
behavior (reference client.end(), client.destroy(), and the existing setTimeout
teardown).
- Around line 108-112: The test "socket that was never read gets destroyed on
native close" incorrectly attaches socket.on('data', ...) which puts the socket
into flowing mode; to properly exercise the "never read" (paused) scenario,
remove the socket.on('data', ...) handler (or explicitly call socket.pause())
inside the callback passed to testServerCloseCompletes so the socket remains
paused; update the code around the anonymous callback used in
testServerCloseCompletes to ensure no data listener is installed and the socket
stays in the paused readable state.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ad3ac33b-56b9-4894-8437-6f0f72c5f2b0

📥 Commits

Reviewing files that changed from the base of the PR and between 010dfa1 and 43b08ba.

📒 Files selected for processing (2)

• src/js/node/net.ts
• test/regression/issue/13184.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/13184.test.ts`:
- Line 38: Replace the string repetition used in the test payload with a
Buffer-based allocation per project guidelines: in the call to client.write(...)
(the line currently using "hello world ".repeat(100)), build the repetitive
payload using Buffer.alloc(count, fill).toString() and pass that string to
client.write; ensure the same repeat count and fill content is used so behavior
is unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 57a6a0ed-1cce-4af0-a142-484bdacb572a

📥 Commits

Reviewing files that changed from the base of the PR and between 43b08ba and 7ce8051.

📒 Files selected for processing (1)

• test/regression/issue/13184.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/13184.test.ts`:
- Around line 37-42: The test races because client.write(...) is followed
immediately by client[teardown]() so server "data" handlers may not run; change
the test to await a server-side confirmation promise before teardown by
creating/waiting on a waitUntilReady (or waitForServerState) Promise that the
server resolves inside the relevant "data" handlers, then after await
clientConnected and client.write(...) do await waitUntilReady before calling
client[teardown]() and awaiting serverSocketClosed—use the existing symbols
client.write, client[teardown], clientConnected, serverSocketClosed and resolve
the promise from the server-side "data" handlers that establish the test state.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8eae5341-f6b8-416d-bc7e-4cb2e50f696a

📥 Commits

Reviewing files that changed from the base of the PR and between 7ce8051 and da62a73.

📒 Files selected for processing (1)

• test/regression/issue/13184.test.ts

[robobun]

Cross-linking #31654, which overlaps this PR's net-level fix (same three close handlers, also drops that // TODO). It passes this PR's paused/unpiped/pipe+pause server.close() scenarios, and additionally (a) handles the write-backpressure variant and (b) keeps Node-faithful error semantics — a peer RST surfaces as 'error' + 'close'(hadError=true) rather than 'end'. Flagging for a maintainer to consolidate; no preference on which lands.