Skip to content

node:http: gate server.close() on connection drain, not request count - #35844

Closed
robobun wants to merge 6 commits into
mainfrom
farm/0d035f14/http-server-close-connection-drain
Closed

node:http: gate server.close() on connection drain, not request count#35844
robobun wants to merge 6 commits into
mainfrom
farm/0d035f14/http-server-close-connection-drain

clear kCloseCallback on re-listen; add server.close() to drain-test f…

0a908e4
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 26, 2026 in 28m 21s

Code review found 1 important issue

Found 1 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/js/node/_http_server.ts:702-706 kRealListen reset before Bun.serve() strands serverSymbol on dead handle if serve throws

Annotations

Check failure on line 706 in src/js/node/_http_server.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

kRealListen reset before Bun.serve() strands serverSymbol on dead handle if serve throws

The reset block clears `kClosing`/`kPendingDrainClose`/`kCloseCallback` before `Bun.serve()` runs, but not `serverSymbol` — so if `Bun.serve()` throws (EADDRINUSE, bad TLS, bad unix path; caught by `listen()`'s try/catch at line 684), `serverSymbol` is left pointing at the already-`stop()`'d OLD handle while `kClosing` is false. OLD's one-shot all-closed promise then resolves into `emitCloseServer`'s `!kClosing` early-return, so a subsequent `close(cb)` sees `serverSymbol` truthy, falls through,