Skip to content

http: avoid stream listeners on idle agent sockets#64004

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:fix-http-agent-free-socket-onread
Jun 20, 2026
Merged

http: avoid stream listeners on idle agent sockets#64004
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:fix-http-agent-free-socket-onread

Conversation

@mcollina

@mcollina mcollina commented Jun 19, 2026

Copy link
Copy Markdown
Member

Fixes: #63989

The response-queue-poisoning guard added to idle http.Agent sockets used a public 'data' listener. That made node-fetch@2 observe socket.listenerCount('data') > 0 during response close and report false ERR_STREAM_PREMATURE_CLOSE errors.

This changes the idle-socket guard to use the socket handle's internal onread hook while the socket is in the free pool, restoring the normal stream read callback when the socket is reused. The guard still destroys sockets that receive unsolicited data while idle, but it no longer adds public stream listeners.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jun 19, 2026
@mcollina

Copy link
Copy Markdown
Member Author

@nodejs/releasers unfortunately, we would need to backport this to node 22.x and 24.x to fix node-fetch@2.

@mcollina mcollina added lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x labels Jun 19, 2026
@mcollina

Copy link
Copy Markdown
Member Author

I'm sorry I should have caught this before shipping.

@Renegade334 Renegade334 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side-note: I had a cursory look for guidance on Node.js internals managing EventEmitter events on emitter-derived Node.js classes, and found none. My feeling is that we don't ordinarily consider changes related to internally-managed event listeners to be semver-major, even though they're publicly observable. Maybe we should elucidate what our stance is, and make it explicit in the EventEmitter documentation?

@Renegade334 Renegade334 added the fast-track PRs that do not need to wait for 72 hours to land. label Jun 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @Renegade334. Please 👍 to approve.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina mcollina force-pushed the fix-http-agent-free-socket-onread branch from 8d8919c to 0d62f61 Compare June 20, 2026 08:30
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 20, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 20, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/64004
✔  Done loading data for nodejs/node/pull/64004
----------------------------------- PR info ------------------------------------
Title      http: avoid stream listeners on idle agent sockets (#64004)
Author     Matteo Collina <[email protected]> (@mcollina)
Branch     mcollina:fix-http-agent-free-socket-onread -> nodejs:main
Labels     http, fast-track, author ready, needs-ci, lts-watch-v22.x, lts-watch-v24.x
Commits    1
 - http: avoid stream listeners on idle agent sockets
Committers 1
 - Matteo Collina <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/64004
Fixes: https://github.com/nodejs/node/issues/63989
Reviewed-By: René <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64004
Fixes: https://github.com/nodejs/node/issues/63989
Reviewed-By: René <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 19 Jun 2026 14:49:05 GMT
   ✔  Approvals: 4
   ✔  - René (@Renegade334): https://github.com/nodejs/node/pull/64004#pullrequestreview-4533753549
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/64004#pullrequestreview-4533914964
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/64004#pullrequestreview-4535587792
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/64004#pullrequestreview-4536884413
   ℹ  This PR is being fast-tracked
   ✘  This PR needs to wait 25 more hours to land (or 0 hours if there is 1 more approval (👍) of the fast-track request from collaborators).
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-06-20T10:50:37Z: https://ci.nodejs.org/job/node-test-pull-request/74300/
- Querying data for job/node-test-pull-request/74300/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/27873727043

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jun 20, 2026
@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 20, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 20, 2026
@nodejs-github-bot nodejs-github-bot merged commit 57a4932 into nodejs:main Jun 20, 2026
100 of 102 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 57a4932

aduh95 pushed a commit that referenced this pull request Jun 20, 2026
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #64004
Fixes: #63989
Reviewed-By: René <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 72 hours to land. http Issues or PRs related to the http subsystem. lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http: node-fetch throws ERR_STREAM_PREMATURE_CLOSE on keep-alive socket closures after latest security releases

8 participants