http: emit drain on socket takeover and avoid stale HWM reuse by trivenay · Pull Request #64991 · nodejs/node · GitHub

When OutgoingMessage transitions from pre-socket buffering (Path B) to socket-connected writing (Path A), the backpressure domain changes. The OM should emit drain at this transition to signal that its buffer is clear and the caller can resume writing under the socket's own backpressure.

Previously, _flush() gated drain emission on writableLength === 0 (which includes socket.writableLength). This conflated the OM's buffer state with the socket's kernel write queue. When the socket had a higher writableHighWaterMark than the OM (e.g., agent reuses a socket from a prior request with a different HWM), the socket was never backpressured, never emitted drain — permanent deadlock.

Approach

This PR makes two changes to address the problem:

1. Drain fix in _flush() (the must-have): Once _flushOutput() completes and all buffered data has been handed to the socket, emit drain unconditionally. From this point, the socket enforces its own backpressure via socket.write() return values. We don't wait for socket.writableLength to reach zero because that's the socket's backpressure domain — not the OM's. If the socket is full, the very next write() through Path A will return false and the user stops writing again naturally.

2. Agent HWM mismatch check (defense in depth): Don't reuse a pooled socket in http.Agent if its writableHighWaterMark differs from the request's highWaterMark. This ensures the user's backpressure threshold is respected for users of the built-in http.Agent. We chose to include this because highWaterMark on a connected TCP socket cannot be changed after creation (the underlying kernel buffer is not exposed via Node's TCP handle, and _writableState.highWaterMark is cosmetic since state.length stays 0 for connected sockets). Since there's no way to make a reused socket respect a different HWM, the most resilient approach is to not reuse it. For requests to the same host:port it's rare that different highWaterMark values are used, so socket reuse still happens for the vast majority of connections.

The drain fix alone prevents the deadlock universally (including custom agents and createConnection). The agent check additionally ensures correct backpressure behavior — not just absence of deadlock — for the common case.

Deadlock reproduction (requires reduced TCP send buffer)

const http = require('http');

const server = http.createServer((req, res) => {
  setTimeout(() => { req.resume(); req.on('end', () => res.end('ok')); }, 30000);
}).listen(0, () => {
  const port = server.address().port;
  const agent = new http.Agent({ keepAlive: true });

  // Request A: creates socket with HWM=10MB
  http.request({ port, method: 'POST', agent, highWaterMark: 10 * 1024 * 1024 }, (res) => {
    res.resume();
    res.on('end', () => {
      setTimeout(() => {
        // Request B: default HWM (64KB), reuses socket (HWM=10MB)
        const req = http.request({ port, method: 'POST', agent });
        // Write 2MB: > 64KB OM HWM, < 10MB socket HWM, > kernel TCP buffer
        const r = req.write(Buffer.alloc(2 * 1024 * 1024));
        if (!r) {
          setTimeout(() => { console.error('DEADLOCK'); process.exit(1); }, 15000);
          req.on('drain', () => req.end());
        } else {
          req.end();
        }
      }, 100);
    });
  }).end('x');
});

sysctl -w net.ipv4.tcp_wmem="4096 16384 65536"
node repro.js  # DEADLOCK without fix, drain fires with fix

Fixes: #64680
Refs: #64653
Refs: #62936

Review requested:

@nodejs/http
@nodejs/net

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.29%. Comparing base (f00fb75) to head (94a47d6).
⚠️ Report is 92 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64991      +/-   ##
==========================================
+ Coverage   90.27%   90.29%   +0.01%     
==========================================
  Files         762      759       -3     
  Lines      247534   247624      +90     
  Branches    46694    46689       -5     
==========================================
+ Hits       223457   223587     +130     
+ Misses      15529    15512      -17     
+ Partials     8548     8525      -23

Files with missing lines	Coverage Δ
lib/_http_agent.js	96.18% <100.00%> (+0.05%)	⬆️
lib/_http_outgoing.js	97.78% <100.00%> (+0.14%)	⬆️

... and 62 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

When OutgoingMessage transitions from pre-socket buffering (Path B) to socket-connected writing (Path A), the backpressure domain changes — subsequent writes go directly to the socket, which enforces its own backpressure via socket.write() return values. The OM should emit drain at this transition point to signal that its buffer is clear and the caller can resume writing under the socket backpressure regime. Previously, _flush() gated drain emission on writableLength === 0 which included socket.writableLength. This conflated two independent backpressure domains: the OM pre-socket buffer and the socket kernel write queue. When the socket had a higher writableHighWaterMark than the OM (e.g. agent-reused socket from a prior request), the socket was never backpressured and never emitted drain, causing a permanent deadlock. Additionally, avoid reusing a pooled socket in http.Agent when its writableHighWaterMark differs from the request highWaterMark, so that the user backpressure threshold is respected for the common case of the built-in Agent. Signed-off-by: Naman Trivedi <trivenay@amazon.com> Fixes: nodejs#64680 Refs: nodejs#64653 Refs: nodejs#62936

CI: https://ci.nodejs.org/job/node-test-pull-request/75624/

lgtm

Commit Queue failed

If this is your first time running this command, follow the instructions to create an access token. If you prefer to create it yourself on Github, see https://github.com/nodejs/node-core-utils/blob/main/README.md. Personal access token auth for Github.

Create a Personal Access Token at https://github.com/settings/tokens

1. Click "Generate new token" → "Generate new token (classic)"
   (fine-grained tokens also work)
2. Set a name, e.g. "my-cli-app"
3. Select scopes: user:email, read:org
4. Generate and copy the token

Paste your token here:

https://github.com/nodejs/node/actions/runs/31247780346

Commit Queue failed

- Loading data for nodejs/node/pull/64991
✔  Done loading data for nodejs/node/pull/64991
----------------------------------- PR info ------------------------------------
Title      http: emit drain on socket takeover and avoid stale HWM reuse (#64991)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     trivenay:http-agent-hwm-no-reuse -> nodejs:main
Labels     http, author ready, needs-ci
Commits    1
 - http: emit drain on socket takeover and avoid stale HWM reuse
Committers 1
 - Naman Trivedi <trivenay@amazon.com>
PR-URL: https://github.com/nodejs/node/pull/64991
Fixes: https://github.com/nodejs/node/issues/64680
Refs: https://github.com/nodejs/node/pull/64653
Refs: https://github.com/nodejs/node/pull/62936
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64991
Fixes: https://github.com/nodejs/node/issues/64680
Refs: https://github.com/nodejs/node/pull/64653
Refs: https://github.com/nodejs/node/pull/62936
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 03 Aug 2026 22:22:17 GMT
   ✔  Approvals: 4
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/64991#pullrequestreview-4876292155
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/64991#pullrequestreview-4878185013
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/64991#pullrequestreview-4887938830
   ✔  - Gürgün Dayıoğlu (@gurgunday): https://github.com/nodejs/node/pull/64991#pullrequestreview-4888441308
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2026-08-07T17:33:14Z: https://ci.nodejs.org/job/node-test-pull-request/75624/
- Querying data for job/node-test-pull-request/75624/
✔  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/31256774273

Looks like the commit-queue failure (✘ GitHub CI is still running) is likely caused by a stale GitHub Actions check suite (ID: 84102428609) from Aug 5 — stuck in queued with 0 check runs, never actually started. I pulled the check suites data locally and simulated the pr_checker.js logic — same result: that one orphaned suite blocks landing while 20 other suites from the same app completed successfully.

For now, I can push an empty commit to get a fresh SHA to unblock this. If there's a way to delete/cancel that stale suite directly, that would work too. #64830 hit the same issue recently and was landed manually with git node land — happy to go whichever route makes sense here.

On the fix side, if a check suite is still queued with 0 runs but a later run of the same app has already completed successfully, it should be ignored. If we think that's the right approach, I can raise a follow-up PR to nodejs/node-core-utils.

Commit Queue failed

- Loading data for nodejs/node/pull/64991
✔  Done loading data for nodejs/node/pull/64991
----------------------------------- PR info ------------------------------------
Title      http: emit drain on socket takeover and avoid stale HWM reuse (#64991)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     trivenay:http-agent-hwm-no-reuse -> nodejs:main
Labels     http, author ready, needs-ci
Commits    1
 - http: emit drain on socket takeover and avoid stale HWM reuse
Committers 1
 - Naman Trivedi <trivenay@amazon.com>
PR-URL: https://github.com/nodejs/node/pull/64991
Fixes: https://github.com/nodejs/node/issues/64680
Refs: https://github.com/nodejs/node/pull/64653
Refs: https://github.com/nodejs/node/pull/62936
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64991
Fixes: https://github.com/nodejs/node/issues/64680
Refs: https://github.com/nodejs/node/pull/64653
Refs: https://github.com/nodejs/node/pull/62936
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 03 Aug 2026 22:22:17 GMT
   ✔  Approvals: 4
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/64991#pullrequestreview-4876292155
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/64991#pullrequestreview-4878185013
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/64991#pullrequestreview-4887938830
   ✔  - Gürgün Dayıoğlu (@gurgunday): https://github.com/nodejs/node/pull/64991#pullrequestreview-4888441308
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2026-08-08T12:19:17Z: https://ci.nodejs.org/job/node-test-pull-request/75624/
- Querying data for job/node-test-pull-request/75624/
✔  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/31291255089

When OutgoingMessage transitions from pre-socket buffering (Path B) to socket-connected writing (Path A), the backpressure domain changes — subsequent writes go directly to the socket, which enforces its own backpressure via socket.write() return values. The OM should emit drain at this transition point to signal that its buffer is clear and the caller can resume writing under the socket backpressure regime. Previously, _flush() gated drain emission on writableLength === 0 which included socket.writableLength. This conflated two independent backpressure domains: the OM pre-socket buffer and the socket kernel write queue. When the socket had a higher writableHighWaterMark than the OM (e.g. agent-reused socket from a prior request), the socket was never backpressured and never emitted drain, causing a permanent deadlock. Additionally, avoid reusing a pooled socket in http.Agent when its writableHighWaterMark differs from the request highWaterMark, so that the user backpressure threshold is respected for the common case of the built-in Agent. Signed-off-by: Naman Trivedi <trivenay@amazon.com> Fixes: #64680 Refs: #64653 Refs: #62936 PR-URL: #64991 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

Landed in 0700e74

On the fix side, if a check suite is still queued with 0 runs but a later run of the same app has already completed successfully, it should be ignored. If we think that's the right approach, I can raise a follow-up PR to nodejs/node-core-utils.

Can you post this bug report in https://github.com/nodejs/node-core-utils, and raise a PR?

Filed the bug report and proposed fix at nodejs/node-core-utils#1160. Started a discussion there on the approach — once we align on the trade-offs, I can raise a PR for the fix.

When OutgoingMessage transitions from pre-socket buffering (Path B) to socket-connected writing (Path A), the backpressure domain changes — subsequent writes go directly to the socket, which enforces its own backpressure via socket.write() return values. The OM should emit drain at this transition point to signal that its buffer is clear and the caller can resume writing under the socket backpressure regime. Previously, _flush() gated drain emission on writableLength === 0 which included socket.writableLength. This conflated two independent backpressure domains: the OM pre-socket buffer and the socket kernel write queue. When the socket had a higher writableHighWaterMark than the OM (e.g. agent-reused socket from a prior request), the socket was never backpressured and never emitted drain, causing a permanent deadlock. Additionally, avoid reusing a pooled socket in http.Agent when its writableHighWaterMark differs from the request highWaterMark, so that the user backpressure threshold is respected for the common case of the built-in Agent. Signed-off-by: Naman Trivedi <trivenay@amazon.com> Fixes: #64680 Refs: #64653 Refs: #62936 PR-URL: #64991 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

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

trivenay mentioned this pull request Aug 3, 2026

http: highWaterMark not respected when agent reuses socket with different HWM #64680

Closed

trivenay force-pushed the http-agent-hwm-no-reuse branch from afb656c to b307aa7 Compare August 3, 2026 22:29

ronag reviewed Aug 4, 2026

View reviewed changes

Comment thread lib/_http_agent.js Outdated

trivenay force-pushed the http-agent-hwm-no-reuse branch from b307aa7 to ce69fc9 Compare August 4, 2026 18:35

ronag reviewed Aug 4, 2026

View reviewed changes

Comment thread lib/_http_outgoing.js Outdated

trivenay force-pushed the http-agent-hwm-no-reuse branch from ce69fc9 to 94a47d6 Compare August 5, 2026 13:19

ronag approved these changes Aug 6, 2026

View reviewed changes

ronag added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Aug 6, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 6, 2026

This comment was marked as outdated.

Sign in to view

ronag requested a review from mcollina August 6, 2026 15:40

This comment was marked as outdated.

Sign in to view

trivikr approved these changes Aug 6, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

jasnell approved these changes Aug 8, 2026

View reviewed changes

gurgunday approved these changes Aug 8, 2026

View reviewed changes

ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 8, 2026

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 8, 2026

panva mentioned this pull request Aug 8, 2026

tools: move ncu config to global for commit queue #65132

Closed

panva removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Aug 8, 2026

panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 8, 2026

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 8, 2026

bjohansebas 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 Aug 9, 2026

bjohansebas approved these changes Aug 9, 2026

View reviewed changes

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 9, 2026

trivikr closed this Aug 9, 2026

trivenay mentioned this pull request Aug 10, 2026

checkGitHubCI blocks indefinitely on orphaned check suites stuck in QUEUED nodejs/node-core-utils#1160

Open

trivenay mentioned this pull request Aug 13, 2026

fix: skip orphaned check suites when checking GitHub CI nodejs/node-core-utils#1163

Open

http: emit drain on socket takeover and avoid stale HWM reuse by trivenay · Pull Request #64991 · nodejs/node · GitHub

Uh oh!

Uh oh!

http: emit drain on socket takeover and avoid stale HWM reuse - #64991

http: emit drain on socket takeover and avoid stale HWM reuse#64991trivenay wants to merge 1 commit intonodejs:mainnodejs/node:mainfrom trivenay:http-agent-hwm-no-reusetrivenay/node:http-agent-hwm-no-reuseCopy head branch name to clipboard

Conversation

trivenay commented Aug 3, 2026

Approach

Deadlock reproduction (requires reduced TCP send buffer)

Uh oh!

nodejs-github-bot commented Aug 3, 2026

Uh oh!

codecov Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Aug 7, 2026

Uh oh!

gurgunday left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 8, 2026

Uh oh!

nodejs-github-bot commented Aug 8, 2026

Uh oh!

trivenay commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 9, 2026

Uh oh!

trivikr commented Aug 9, 2026

Uh oh!

trivikr commented Aug 9, 2026

Uh oh!

trivenay commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

http: emit drain on socket takeover and avoid stale HWM reuse#64991
trivenay wants to merge 1 commit into
nodejs:mainfrom
trivenay:http-agent-hwm-no-reuse

codecov Bot commented Aug 4, 2026 •
edited
Loading

trivenay commented Aug 8, 2026 •
edited
Loading