http: close pre-request sockets in closeIdleConnections#63470
Open
semimikoh wants to merge 1 commit into
Open
Conversation
Collaborator
|
Review requested:
|
Signed-off-by: semimikoh <ejffjeosms@gmail.com>
2fa5144 to
95c435e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #63470 +/- ##
==========================================
- Coverage 91.67% 90.05% -1.62%
==========================================
Files 361 714 +353
Lines 156408 225925 +69517
Branches 24050 42732 +18682
==========================================
+ Hits 143384 203463 +60079
- Misses 12751 14238 +1487
- Partials 273 8224 +7951
🚀 New features to boost your workflow:
|
metcoder95
approved these changes
May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
server.closeIdleConnections()does not close TCP connections that have been accepted but have not yet sent any HTTP data, contradicting itsdocumented 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:
Before:
still hung after 2s. After: closes immediately.Reported in #63452.
Cause
New sockets are pushed into
kConnectionsfromconnectionListenerInternalviaparser.initialize(...), butParser::Initialize(
src/node_http_parser.cc:730) setslast_message_start_ = uv_hrtime()so thatheadersTimeout/requestTimeoutcan begin counting (DoSprotection).
ConnectionsList::Idle()(src/node_http_parser.cc:1147) only returns parsers withlast_message_start_ === 0, which becomes trueonly after
on_message_completeresets it (line 544). Sockets that were accepted but have not sent any HTTP data are therefore classified as activeand skipped by
closeIdleConnections().Fix
lib/_http_server.js: in addition to the existingidle()iteration, walkall()and destroy any socket whosebytesRead === 0. That uniquelyidentifies sockets that have not received any data and so cannot be sending a request or awaiting a response. Keep-alive sockets between requests,
in-flight requests, and in-flight responses all have
bytesRead > 0and are unaffected.Test
Added
test/parallel/test-http-server-close-idle-connections-pre-request.js. It opens a TCP connection without sending any data, callsserver.close()+server.closeIdleConnections(), and uses an unref'dsetTimeout(common.mustNotCall(), 1000)as the safety net: if the fix worksthe event loop drains and the timer never fires; without the fix the timer fires and the assertion trips.
Verification
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 behavior change was verified by monkey-patchingServer.prototype.closeIdleConnectionswith the same logic and running the issue's reproduction.Fixes: #63452