René's URL Explorer Experiment

Title: http: close pre-request sockets in closeIdleConnections by semimikoh · Pull Request #63470 · nodejs/node · GitHub

Open Graph Title: http: close pre-request sockets in closeIdleConnections by semimikoh · Pull Request #63470 · nodejs/node

X Title: http: close pre-request sockets in closeIdleConnections by semimikoh · Pull Request #63470 · nodejs/node

Description: Problem server.closeIdleConnections() does not close TCP connections that have been accepted but have not yet sent any HTTP data, contradicting its documented behavior ("Closes all connections connected to this server which are not sending a request or waiting for a response") and blocking graceful shutdown when peers (e.g. browsers opening speculative connections) hold sockets open without writing. Repro: import { createServer } from 'node:http'; import { createConnection } from 'node:net'; const server = createServer((req, res) => res.end('ok')); server.listen(3099, '127.0.0.1'); await new Promise((r) => server.once('listening', r)); const sock = createConnection(3099, '127.0.0.1'); await new Promise((r) => sock.once('connect', r)); await new Promise((r) => server.once('connection', r)); const closed = new Promise((r) => server.close(() => r(performance.now()))); const closeStart = performance.now(); server.closeIdleConnections(); const t = await Promise.race([ closed, new Promise((r) => setTimeout(() => r(null), 2000)), ]); console.log(t === null ? 'still hung after 2s' : `closed in ${t - closeStart}ms`); sock.destroy(); Before: still hung after 2s. After: closes immediately. Reported in #63452. Cause New sockets are pushed into kConnections from connectionListenerInternal via parser.initialize(...), but Parser::Initialize (src/node_http_parser.cc) sets last_message_start_ = uv_hrtime() so that headersTimeout / requestTimeout can begin counting (DoS protection). ConnectionsList::Idle() only returns parsers with last_message_start_ === 0, which becomes true only after on_message_complete resets it. Sockets that were accepted but have not sent any HTTP data are therefore classified as active and skipped by closeIdleConnections(). Fix src/node_http_parser.cc: add a received_data_ flag on Parser. It is reset to false in Parser::Initialize (for server-side parsers that own a ConnectionsList) and set to true in on_message_begin, when llhttp signals the first byte of a new HTTP message. ConnectionsList::Idle() now also returns parsers where received_data_ is still false, so accepted-but-pre-request sockets are reported as idle and closed by server.closeIdleConnections(). Per @pimterry's review suggestion, the fix lives at the parser layer rather than as a JS-side filter, so the semantics of idle() are correct for any current or future caller. received_data_ is not part of ParserComparator's ordering key, so it can be updated in place without pop/push around the std::set. Test Added test/parallel/test-http-server-close-idle-connections-pre-request.js. It opens a TCP connection without sending any data, calls server.close() + server.closeIdleConnections(), and uses an unref'd setTimeout(common.mustNotCall(), 1000) as the safety net: if the fix works the event loop drains and the timer never fires; without the fix the timer fires and the assertion trips. Verification The behavior change was first validated by reproducing the fix's semantics in JS (returning pre-request sockets from idle() equivalent) and running the issue's repro, which closed in ~0.4ms instead of hanging. A full local build of the patched binary was not completed because this machine has Apple clang 16.0.0, while the current tree requires a newer toolchain (V8 fails to compile due to missing std::atomic_ref). The C++ change is small and the semantics are 1:1 with the validated JS-equivalent behavior; CI will confirm. Fixes: #63452

Open Graph Description: Problem server.closeIdleConnections() does not close TCP connections that have been accepted but have not yet sent any HTTP data, contradicting its documented behavior ("Closes all connections...

X Description: Problem server.closeIdleConnections() does not close TCP connections that have been accepted but have not yet sent any HTTP data, contradicting its documented behavior ("Closes all connect...

Opengraph URL: https://github.com/nodejs/node/pull/63470

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width