[dennev]

When the event loop is blocked long enough for a free socket's idle timeout to expire, its timeout callback cannot run. The next request can then reuse that socket from agent.freeSockets even though the server may already have closed the connection, causing the request to fail with ECONNRESET.

This change addresses the timeout-expiry case discussed in #54293. In addRequest(), the agent reads the timers' clock (timersBinding.getLibuvNow()) once and checks each free candidate against its idle timer (socket[kTimeout]), which records on the same clock when it was last started or refreshed. Expired candidates are removed and destroyed until an unexpired socket is found or the pool is empty. A socket without a timeout (socket.timeout is 0), or a custom stream that does not have this timer, is never expired this way.

Following review, a second commit also keeps addRequest() from handing out a free socket that is already destroyed, wherever it is in the pool. A destroyed candidate is removed from the agent at once, like an expired one. Destroyed sockets at the head of the pool are still dropped by the existing loop, as before, and stay counted in agent.totalSocketCount until their 'close' event. A third commit adds a test for a pool that holds an expired and an unexpired socket, and a fourth, following a review suggestion, reads the expiry from the socket's idle timer instead of a timestamp recorded when the socket entered the pool.

The clock call preserves the binding object as its receiver, as required by the fast API call. The regression test also exercises an optimized addRequest().

Not covered by this change:

• free sockets without an idle timeout, including those belonging to agents without a timeout (http: Agent with keepAlive and no timeout ignores the server's Keep-Alive timeout hint #66734, http: respect Keep-Alive timeout hint when agent has no explicit timeout #66779)
• a server close still in flight when the request is sent (the general keep-alive race discussed in Network request fails after event loop is blocked for a few seconds (regression in v20) #54293)
• a socket handed directly from the 'free' handler to a queued request
• system suspend
• synchronous blocking before the socket enters the free pool, for example in a response's 'data' or 'end' handler: with an agent timeout, the client clears the socket's timeout when it keeps the socket alive, and the agent restarts the idle timer as the socket enters the pool, so time spent blocked before that point is not counted

Testing: I also ran a reproduction based on #54293, with server.keepAliveTimeout = 5000 and a 6-second block between two requests through http.globalAgent. It fails with ECONNRESET on v26.12.1 and succeeds with the first commit, using a new socket.

Agent-run validation confirmed that the regression test fails without this change and passes with it. It covers LIFO and FIFO expiry, reuse before expiry, unchanged reuse without an idle timeout, maxSockets accounting after a block, a pool that holds an expired and an unexpired socket (LIFO and FIFO), a pooled socket whose timeout is set again, a request timeout that fired and was cleared, a destroyed socket at the tail of the pool with LIFO, and optimized addRequest().

Agent-run validation at the current head (Linux, release build): the regression test, the HTTP/HTTPS and agent-related tests and the full make -j4 test passed, and the regression test also passed on a debug build, where its fast API call assertion runs. In agent runs of the same cases, the mixed-pool case fails without the first commit, the destroyed-socket case fails without the second, and the restarted-timeout case fails without the fourth. An agent-run reproduction based on #54293 also fails with ECONNRESET on v26.13.1 (official Docker image, Linux).

AI disclosure: Claude Code (Claude Opus 5.5) was used to investigate the cause, write and review the code, test and documentation changes, and run the agent-run validation described above. Before opening this pull request, Claude and Codex (gpt-6.1-sol) reviewed the first commit, its commit message and the description as first posted. The second and third commits were prepared the same way with Claude Code (Claude Fable 5.2), and the fourth with Claude Code (Claude Opus 5.5). Claude reviewed the later commits and their messages before I pushed them, and drafts of my comments and of this updated description before I posted them; Codex also reviewed the third and fourth commits.

Refs: #54293

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/net

[github-actions]

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

• the guide for first-time contributors
• the contribution and automation policies
• the pull request guide
• the AI use policy
• the Code of Conduct

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

[mcollina]

lgtm

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.70%. Comparing base (bf246cf) to head (13a482f).
⚠️ Report is 156 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##             main   #67292     +/-   ##
=========================================
  Coverage   90.69%   90.70%             
=========================================
  Files         797      802      +5     
  Lines      278787   282134   +3347     
  Branches    53610    54364    +754     
=========================================
+ Hits       252849   255900   +3051     
- Misses      16310    16368     +58     
- Partials     9628     9866    +238     

Files with missing lines	Coverage Δ
lib/internal/http/agent.js	96.75% <100.00%> (+0.11%)	⬆️

... and 143 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.

[nodejs-github-bot]

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

[gurgunday]

Suggested change

	if (!isFreeSocketExpired(socket, now))
	if (!socket.destroyed && !isFreeSocketExpired(socket, now))

Since this loop now walks the pool anyway, could it also skip sockets that are already destroyed? This will fix another bug

In LIFO mode a request made from setImmediate in that window still pops it and fails with econnreset

[dennev]

Thanks, good catch. Applied in a1d6973, together with a test case for it.

I reproduced it on v26.12.1: with two sockets in the free pool and the default lifo scheduling, a request made right after the most recently freed socket is destroyed fails with ECONNRESET (socket hang up).

I ran the updated test on a release build on Linux, and it passed. Agent-run validation: the new case fails without the change; with it, the regression test, the HTTP/HTTPS tests and the full make -j12 test passed.

[dennev]

I pushed one more commit, test only (13a482f). It adds a case, for lifo and for fifo, in which the socket that the scheduling picks first has expired and the other one has not: the expired socket is removed and the request reuses the other one. Until now no case had a pool that holds an expired and an unexpired socket.

I ran the updated test on a release build on Linux, and it passed.

Agent-run validation: the new case fails without the first commit of this PR and passes with it; the regression test, the HTTP/HTTPS tests and the full make -j4 test passed. The updated test also passed on a debug build, where its fast API call assertion runs.

[mcollina]

Instead of tracking a separate kFreeSince timestamp, could we read the socket's idle timer directly? It already records when it was last armed or refreshed:

const { kTimeout } = require('internal/timers');

function isFreeSocketExpired(socket, now) {
  const timer = socket[kTimeout];
  return timer != null && timer._idleTimeout > 0 &&
    now - timer._idleStart >= timer._idleTimeout;
}

This removes the getLibuvNow() call and the extra property write from the 'free' handler, so the only new cost on the reuse path is the single clock read in addRequest().

It's also closer to the timer it's emulating. _idleStart is refreshed on every read and write (Socket.prototype._unrefTimer). When keepSocketAlive() doesn't call setTimeout() because socket.timeout already matches, the timer keeps counting from the last I/O, which is close to when the server's keep-alive timer started. That covers the "blocked in 'data'/'end' before the socket enters the pool" case listed as not covered. When setTimeout() is called, _idleStart is reset at that moment, which matches kFreeSince today.

The _idleTimeout > 0 check also handles a cleared timer. setStreamTimeout(0) now keeps the cleared Timeout in this[kTimeout] for reuse, and unenroll() sets its _idleTimeout to -1.

[dennev]

Thanks, applied in 9ac5f02. isFreeSocketExpired() now reads socket[kTimeout], and the 'free' handler no longer reads the clock or writes kFreeSince. I also reworded the doc sentence to "a free socket whose timeout has passed", since the time spent in the pool is no longer what is measured.

One difference from your snippet: I kept the socket.timeout > 0 check. A timer that has fired keeps its _idleTimeout when it is cleared afterwards, because unenroll() returns early for a destroyed timer (lib/timers.js). Without the check, a socket with no timeout whose request timeout had fired while the response waited unread would count as expired and not be reused. The new testFiredTimeoutCleared case covers this; testRestartedTimeout covers a pooled socket whose timeout is set again, which the previous check expired from the time it entered the pool.

One trade-off of reading the timer: a stream from a custom createConnection() that keeps its own timeout without this timer is no longer expired by the clock; with the previous check it was, if it exposed timeout.

One caveat on the coverage: the "blocked in 'data'/'end'" case is still not covered in the client's own flow. When the request or the agent has a timeout, responseKeepAlive() clears the socket's timer with socket.setTimeout(0, req.timeoutCb) (lib/internal/http/client.js). With an agent timeout, keepSocketAlive() then sees socket.timeout === 0 and calls setTimeout() again as the socket enters the pool, which resets _idleStart there. Agent-run check: with a 300 ms block in the 'end' handler of a request through new Agent({ keepAlive: true, timeout: 100 }), the next request still reuses the socket with this change, so I kept that case under "Not covered" in the description.

Agent-run validation: testRestartedTimeout fails without this commit, testFiredTimeoutCleared fails without the socket.timeout > 0 check, and both pass with it; the regression test passed 200 of 200 runs, the HTTP/HTTPS and agent-related tests and the full make -j4 test passed, and the regression test also passed on a debug build, where its fast API call assertion runs.