fix: run idle socket validation off setImmediate instead of setTimeout by carlotestor · Pull Request #5499 · nodejs/undici

carlotestor · 2026-07-03T12:26:37Z

Summary

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

carlotestor · 2026-07-03T12:26:37Z

Summary

Fixes#5493

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

carlotestor · 2026-07-03T12:26:37Z

Summary

Fixes#5493

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

carlotestor · 2026-07-03T12:26:37Z

Summary

Fixes#5493

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

carlotestor · 2026-07-03T12:26:37Z

Summary

Fixes#5493

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

carlotestor · 2026-07-03T12:26:37Z

Summary

Fixes#5493

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

carlotestor · 2026-07-03T12:26:37Z

Summary

Fixes#5493

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

carlotestor · 2026-07-03T12:26:37Z

Summary

Fixes#5493

Schedule the idle-socket validation deferral with setImmediate instead of setTimeout(..., 0) in scheduleIdleSocketValidation (lib/dispatcher/client-h1.js), and cancel it with clearImmediate accordingly (clearTimeout does not cancel an Immediate, so the pending callback would otherwise fire after socket cleanup — harmlessly, since it self-guards, but the handle should be cancelled properly). The now-meaningless 0 delay argument is dropped.

The validation only needs the event loop to surface any unsolicited bytes / FIN / RST already pending on the reused idle socket before the next request is written. The setImmediate check phase still runs after the poll phase, so those events are processed first — but without the ~1ms+ timer floor that setTimeout(0) pays on every idle-socket reuse.

Measured

Loopback HTTP, keep-alive Agent (pipelining: 1), 2000 sequential requests + 8-wide bursts, node v22.22.1 (full harness in #5493):

	sequential p50	8-wide burst p50
main (`setTimeout(0)`)	~1.4 ms	~1.9 ms
this patch (`setImmediate`)	0.076 ms	0.446 ms
7.27.2 (pre-validation baseline)	0.078 ms	0.49 ms

i.e. the GHSA-35p6-xmwp-9g52 mitigation is retained at effectively zero latency cost.

Tests

node --test test/response-queue-poisoning.js test/issue-810.js test/client-keep-alive.js — 15/15 pass with the patch (including the poisoned-idle-socket regression test)

Notes

The kIdleSocketValidationTimeout symbol now holds an Immediate; left the name unchanged to keep the diff minimal, happy to rename if preferred.
Immediate.unref() exists, so the existing .unref?.() call keeps working.

Disclosure

This pull request (analysis, patch, benchmarks, and text) was created by Claude (model: claude-fable), an AI assistant by Anthropic, operating under human direction. Please review accordingly.

mcollina

lgtm

codecov-commenter · 2026-07-11T16:05:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.43%. Comparing base (3d78e1a) to head (137d592).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #5499 +/- ##
=======================================
Coverage 93.43% 93.43% =======================================
Files 110 110 Lines 37278 37278 =======================================
Hits 34830 34830 Misses 2448 2448

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

…on idle event loop (#5606) * fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop The change from setTimeout(0) to setImmediate() in #5499 introduced a regression where reusing an idle keep-alive connection could stall for ~500ms. The root cause is the interaction between setImmediate().unref() and socket.unref() in the idle state: when the socket is unref'd and the only remaining handle is an unref'd setImmediate, the event loop poll phase waits for its timeout before reaching the check phase. setTimeout(0) fires in the timers phase (before poll), avoiding this problem entirely at the cost of ~1ms timer resolution per reuse. Fixes#5600 * test: add keep-alive socket reuse stall regression test * test: assert keep-alive reuse progress via logical I/O events Replace wall-clock timing and artificial server delays with server connection/request event coordination. The test timeout catches the setImmediate poll-phase stall regression without flaky Date.now checks. Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>

Reusing an idle keep-alive socket can stall for up to a fast-timers tick (~500ms) before the request reaches the wire. With a pool of one connection and a 10ms server, six sequential requests take over a second instead of the ~60ms the server accounts for. `scheduleIdleSocketValidation` moved from an unref'd `setTimeout(..., 0)` to `setImmediate` in nodejs#5499, and the `unref?.()` came along with it. An unref'd immediate does not hold the loop open, so on an otherwise idle loop the check phase can be reached only once the next fast-timers tick wakes things up, and the queued request waits for it. The request the validation exists for is already queued at that point, so there is nothing to keep alive artificially: the immediate is a one-shot that runs on the very next check phase and `clearIdleSocketValidation` cancels it when the socket goes away. test/issue-5600.js fails on the current code with a slowest request around 425ms and passes with the immediate left ref'd; on this machine the six requests go from 1095/1565/2045ms across runs to 122/137/219ms. Signed-off-by: marko1olo <marko1olo@users.noreply.github.com>

fix: run idle socket validation off setImmediate instead of setTimeout
137d592

carlotestor force-pushed the fix/idle-validation-setimmediate branch from 8dcfcfa to 137d592Compare July 3, 2026 12:33

carlotestor mentioned this pull request Jul 3, 2026
feat(base): replace node-fetch with native fetch (undici), HTTP/1.1 keep-alive pooling ccxt/ccxt#29084
Merged
7 tasks

ronag mentioned this pull request Jul 11, 2026
perf(h1): reduce idle validation latency nxtedition/undici#22
Merged

mcollina approved these changes Jul 11, 2026
View reviewed changes

mcollina merged commit ecf714f into nodejs:mainJul 11, 2026
36 of 38 checks passed

github-actionsBot mentioned this pull request Jul 20, 2026
[Release] v8.8.0 #5575
Merged

marceli1404 mentioned this pull request Jul 28, 2026
fix: revert idle socket validation to setTimeout(0) to prevent stall on idle event loop #5606
Merged

marko1olo mentioned this pull request Jul 29, 2026
fix(h1): stop unref'ing the idle socket validation immediate #5609
Closed
6 tasks

ostapondo mentioned this pull request Aug 3, 2026
CI red on all PRs since Aug 1: Node 26.5.1 (undici 8.8+) dispatch stall breaks "update state while run in flight" langchain-ai/langgraphjs#2648
Closed

ronag mentioned this pull request Aug 8, 2026
fix: port remaining applicable upstream fixes (timer guards, dead pool path) nxtedition/undici#68
Merged

This was referenced Aug 18, 2026
perf(fetch): skip empty RequestInit work #5701
Merged
perf(h1): drop idle-socket timer floor with a ref'd setImmediate #5707
Merged