node:http: emit 'pause' on req.socket once an unread body fills the IncomingMessage buffer - #34740
Conversation
…ressure The native server dispatcher paused the body handle on arrival and only installed ondata from IncomingMessage._read(), so a handler that never read the body never saw body bytes reach the IncomingMessage and req.socket never had pause() called on it. Node's connectionListener feeds body bytes through parserOnBody unconditionally and calls readStop(socket) when push() returns false, which is what emits 'pause' on the connection. Install ondata at dispatch (replacing the unconditional handle.pause()) and have the push callback readStop(this.socket) once the Readable buffer fills. NodeHTTPServerSocket.pause() forwards to response.pause() as before, so kernel reads still stop, and Duplex.pause() emits the 'pause' event code like test-http-no-read-no-dump keys on. This also bounds IncomingMessage.readableLength when the handler reads slowly (was growing unbounded), since the native ondata path now backpressures instead of ignoring push()'s return.
|
Updated 2:51 AM PT - Jul 20th, 2026
❌ @robobun, your commit f996a40 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 34740That installs a local version of the PR into your bun-34740 --bun |
WalkthroughChangesHTTP request body delivery now shares a centralized data handler that applies backpressure. Response callback re-arming and socket shutdown pauses are state-gated, with regression tests covering unread bodies, pause events, and keep-alive sequencing. HTTP body backpressure
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
…e the body is delivered The eager onData install exposed a pipelining hazard: a pipelined request's _read() reaches socket.resume() -> #resumeSocket() -> currentResponseObject.resume(), which is the in-flight request's handle. do_resume() unconditionally re-registered onData and onTimeout, both of which overwrite the per-socket HttpResponseData userData, so the pipelined request's body fin was dispatched to the previous request's handle and picked up its trailers. Once body_read_state has left Pending there is nothing to re-arm, so gate the re-registration on it. resume_socket() still runs so a deferred FIN on a paused fd is delivered.
…heir own file For an accepted Upgrade with a body, the upgrade listener owns the socket's flow state for the tunnel bytes that follow the body, so readStop(socket) from the request's push callback (mirroring parserOnBody) must not touch it. _read() already special-cases upgrade for the same reason. Without this the tunnel bytes stalled behind a paused socket on Windows, where do_pause() does not stop kernel reads and the body fills the Readable buffer in-process. Move the two new 'pause' tests to their own file so the gate's with-fix run is not tripped by the pre-existing 'request via http proxy' failure in node-http.test.ts.
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
readStop(socket) from a pipelined request's body push reaches socket.pause() -> currentResponseObject.pause() where currentResponseObject is still the previous in-flight request's handle. Without this guard do_pause on that handle re-arms on_buffer_paused_shim with its ctx and steals the pipelined request's body bytes. Symmetric with the do_resume guard.
|
Ready for review at f996a40. The darwin-26 CI red on #76168 is unrelated: |
…Pending pausePipelineReads (_http_server.ts) calls response.pause() on the head in-flight request, whose body_read_state is necessarily None or Done by the time a pipelined request is dispatched. The body_read_state guard added in aafc38d returned before pause_socket(), so the write-side read gate set socket._paused=true in JS but never stopped kernel reads. Gate only the onData re-arm on body_read_state; pause_socket() runs regardless (mirroring do_resume, which resume_socket()s before its identical check).
There was a problem hiding this comment.
No new issues found; my three earlier rounds of feedback are all addressed in a315c2d (the body_read_state guard now wraps only the on_data re-arm, so pause_socket() still runs for pausePipelineReads, and do_pause/do_resume are symmetric). This is a substantive change to the server's body-flow model — eager push at dispatch instead of lazy install from _read(), plus native guards around shared HttpResponseData state under pipelining — so it's worth a human look, but I have nothing further.
What was reviewed
onDataIncomingMessage→readStop(socket)routing under pipelined dispatch (currentResponseObjectpoints at the head request) — the newbody_read_stateguards on bothdo_pauseanddo_resumeprevent stale-ctx re-arm.pausePipelineReadsstill reachespause_socket()after a315c2d;resume_socket()remains unconditional.- Upgrade-with-body path skips
readStop, matching the existing_read()carve-out.
Extended reasoning...
Overview
This PR changes the native node:http server so request-body bytes are pushed into the IncomingMessage eagerly at dispatch (via handle.ondata = onDataIncomingMessage.bind(http_req)) instead of only after _read() runs. onDataIncomingMessage now calls readStop(this.socket) when push() returns false, which routes through NodeHTTPServerSocket.pause() → native do_pause and emits 'pause' on the socket — matching Node's parserOnBody behavior. On the Rust side, do_pause and do_resume gain a body_read_state == Pending gate around the raw.on_data(...) re-arm so a pause/resume routed to a request whose body is already delivered can't overwrite a pipelined successor's ctx on the shared uWS HttpResponseData.
Prior review rounds
I raised three issues on earlier revisions, all now resolved:
readStop(socket)on a pipelined request reaches the previous request's handle viacurrentResponseObject, whosedo_pausere-armedon_buffer_paused_shimwith the wrong ctx → fixed by thebody_read_stateguard indo_pause(aafc38d).do_pause/do_resumeasymmetry (same rationale) → same commit.- That guard's early-return skipped
pause_socket(), defeatingpausePipelineReads→ fixed in a315c2d by scoping the guard to only theon_datare-arm.
I re-verified the final shape at NodeHTTPResponse.rs:1339-1439: the four-flag early-return is unchanged from main; body_read_state == Pending wraps only the IS_DATA_BUFFERED_DURING_PAUSE + raw.on_data block; pause_socket() runs regardless; do_resume calls resume_socket() unconditionally before its guard. Symmetric.
Security risks
None identified. This is flow-control on request-body reads; no auth, crypto, or trust-boundary parsing is touched. The change adds backpressure (bounds req.readableLength at the highWaterMark instead of unbounded growth), which is a hardening.
Level of scrutiny
High. This changes when body bytes reach the IncomingMessage for every native-server request with a body, and the pipelining interaction with uWS's shared per-socket userData slot is subtle enough that it took three review iterations to land correctly. The author also notes a pre-existing pipelined-POST-body bug on main that this PR doesn't fix (deferred to a follow-up). A maintainer familiar with the NodeHTTPResponse / uWS callback-registration model should confirm the eager-install doesn't regress any path the existing test sweep doesn't cover.
Other factors
Two new targeted tests plus Node's upstream test-http-no-read-no-dump.js are added; the PR description reports the 376-file test-http-* sweep and the express/body-parser integration tests are unchanged or improved. The robobun evidence gate's "without fix" runs appear to have used in-PR commits (e2b534f / aafc38d) rather than main, so its fail-on-main proof is not clean, but the mechanism is well-explained and the debug-build timeout on main is consistent with the described root cause.
socketHandle.end() (res.socket.end() -> _final()) calls us_socket_resume() after the shutdown specifically so kqueue's one-shot EVFILT_WRITE delivers EV_EOF and the unread body drains. With the eager body push, a readStop(socket) from the first body chunk that arrives after that end() re-paused the poll, re-deferring the EOF on kqueue so 'close' never fired (macOS 26 only in CI; darwin-14 passes). writableEnded is the Duplex flag end() sets before _final(), so gating readStop on it leaves the poll armed exactly when socketHandle.end() needs it.
…dy-done predicate on_buffer_request_body_while_paused sets that flag when the last chunk arrives via the pause buffer but leaves body_read_state at Pending; set_on_data and get_has_body already treat the flag as part of the body-done invariant. do_pause/do_resume now match: the onData re-arm is gated on Pending && !LAST, and do_resume's drain runs regardless so a body buffered-while-paused still reaches its caller.
…rt on body completion; route upgrade backpressure through the request's own handle Three follow-ups from review and the darwin-26 regression: socketHandle.end()'s us_socket_resume() was written to undo onNodeHTTPRequest's dispatch-time handle.pause(), which this PR removed. With the socket never paused, us_socket_resume() early-returns and the kqueue EVFILT_READ delete/re-add cycle that us_internal_socket_raw_ shutdown relied on never happens, so on macOS 26 a res.socket.end() mid-upload left the server in FIN_WAIT_2 with the peer's close never delivered (node-http-halfclose-midupload.test.ts). us_socket_pause() before the buffered write restores the cycle regardless of dispatch state; verified 8/8 CLOSED on a macOS 26.4 CI host via dtrace. onDataIncomingMessage's isLast branch now calls readStart(socket) after emitEOFIncomingMessage, mirroring Node's parserOnMessageComplete and Bun's own llhttp path, so a readStop from the last body chunk does not leave the shared socket's flowing=false for the next keep-alive request. New test covers two body-bearing POSTs on one connection. Upgrade-with-body routes backpressure through this[kHandle].pause() instead of skipping it entirely, restoring the fd-level bound the dispatch-time handle.pause() used to provide; _read()'s onIncomingMessageResumeNodeHTTPResponse already balances it.
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
Do not add user-facing non-public exports or properties. Put it in an internal/ module if needed.
… tighten keep-alive test
onDataIncomingMessage was exported from node:_http_incoming which is a
public module; move the backpressure-aware version to internal/http.ts
(which already has the symbols it needs) and revert _http_incoming.ts's
copy to its previous shape. _http_server.ts imports it from internal/http.
Add extern "C" void us_socket_pause(us_socket_t*) alongside the existing
us_socket_resume declaration so the file compiles without unified-source
bundling pulling libusockets.h in first.
Keep-alive test: use .once('pause') so the per-request listener does not
leak onto the shared socket, and assert the exact [/a, /b] sequence;
import Agent at module scope instead of inline require().
|
Moved |
…ad of keeping a divergent copy _read()'s !handle.ondata fallback now binds the same backpressure-aware implementation the dispatcher installs, removing the duplicate that had diverged to the pre-PR shape.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/node/http/node-http-req-socket-pause.test.ts`:
- Around line 78-80: Remove the explanatory regression rationale comment near
the request socket pause test, leaving only the permitted issue URL comment if
one exists. Preserve the test behavior and move no additional rationale into
code comments.
- Around line 83-108: Update the test around the server callback to capture each
request’s req.socket while recording pauses, then assert the sockets for "/a"
and "/b" are the same object. Keep the existing pause-order assertion and
cleanup, ensuring the test specifically verifies keep-alive agent reuse rather
than allowing a reconnect to satisfy it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9fd3e714-dd3f-45de-bf3a-6d7578395c01
📒 Files selected for processing (6)
src/js/internal/http.tssrc/js/node/_http_incoming.tssrc/js/node/_http_server.tssrc/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cppsrc/runtime/server/NodeHTTPResponse.rstest/js/node/http/node-http-req-socket-pause.test.ts
Without the socket-identity check, a reconnect would let /b emit 'pause' even if the shared-socket flowing state regressed. Drop the explanatory comment since the test name already describes the scenario.
There was a problem hiding this comment.
No new findings after c34fb04 and f996a40 — the duplicate onDataIncomingMessage is gone and the keep-alive test now asserts socket identity. This is still a behavioral change to the native node:http server's request-body flow (lazy pause-at-dispatch → eager push) across JS builtins, Rust uWS callback re-arm guards, and a macOS-specific kqueue pause/resume cycle in C++, so it's worth a human look before merge.
What was reviewed:
do_pause/do_resumenow gate only theon_datare-arm onbody_read_state == Pending && !IS_DATA_BUFFERED_DURING_PAUSE_LAST, matchingset_on_data;pause_socket()/buffer drain still run unconditionally._http_incoming.tsnow imports the sharedonDataIncomingMessage;_read()'s!handle.ondatafallback binds the same implementation.- The
us_socket_pauseextern-C declaration is present; the keep-alive test uses.once+toEqualand assertssockets[0] === sockets[1]. - The pause-buffer completion route (
on_buffer_paused_shimdeliveringlast=true) is acknowledged for the pipelining follow-up.
Extended reasoning...
Overview
The PR switches the native node:http server from pausing the body handle at dispatch to installing handle.ondata eagerly, so body bytes flow into the IncomingMessage unconditionally and push() returning false triggers socket.pause() on the connection — matching Node's parserOnBody → readStop model. It touches src/js/internal/http.ts (new shared onDataIncomingMessage), _http_incoming.ts (deletes the divergent copy), _http_server.ts (dispatch change), NodeHTTPResponse.rs (do_pause/do_resume guards so a completed request's resume cannot overwrite a pipelined request's uWS userData), and JSNodeHTTPServerSocketPrototype.cpp (an explicit us_socket_pause/us_socket_resume cycle around the shutdown to keep macOS 26 kqueue delivering the peer close now that dispatch no longer pauses).
Security risks
None identified. This is flow-control/backpressure semantics on the request body Readable; no auth, crypto, or input validation surface changes. The readStop addition bounds req.readableLength (previously unbounded on this path), which is a resource-consumption improvement.
Level of scrutiny
High. This is production-critical code — every node:http server request with a body goes through the changed dispatch path — and the fix required nine review iterations with substantive corrections at each (the body_read_state guard scope, the IS_DATA_BUFFERED_DURING_PAUSE_LAST predicate match, the upgrade-with-body branch, the readStart on isLast, the missing extern declaration, the duplicate helper). The C++ change encodes a macOS-26-specific kqueue EVFILT_READ behavior in a comment; that platform interaction is not directly testable in CI on other platforms.
Other factors
All prior inline findings (mine and CodeRabbit's) are resolved or explicitly deferred. The one open item — the on_buffer_paused_shim completion route not running socket.resume() — was filed as a nit and the author confirmed it overlaps a pre-existing pipelined-POST-body bug already handed off for a separate follow-up; the sequential keep-alive case is covered by the _dump() → _read() fallback and the new keep-alive test. Test coverage is good (three new tests plus Node's upstream test-http-no-read-no-dump.js, and the PR body reports the 376-file test-http-* sweep clean). Given the breadth of interacting subsystems and the iteration history, a maintainer sign-off is appropriate.
…aused (#37977) ### Problem - On Windows, a `node:http` server whose handler stops reading the request body (`req.pause()`, or simply not consuming `req`) keeps accepting the upload at full speed; the bytes pile up in native memory until the request is resumed. Same scenario as #26332, which was filed from Windows: #34740 bounded the JS-side `IncomingMessage` buffer on every platform, but on Windows that only moved the growth into the native pause buffer. - Repro below, 2.5 s after `req.pause()` (loopback upload of 256 KiB chunks): Windows x64 canary `9a543cc18` has pulled 8195 chunks (2 GB) and RSS is 2.1 GB and climbing; Linux stays at 12 chunks and 37 MB; Node v26 on Windows stays at 15 chunks. - A client that finishes its upload and half-closes while the request is paused also gets its request aborted on Windows (the body was parked natively, so `'end'` never fired before the FIN arrived); Node and Bun on Linux deliver the body and the response. - Cause: `NodeHTTPResponse::do_pause` (`src/runtime/server/NodeHTTPResponse.rs`) re-arms uWS `onData` with `on_buffer_paused_shim`, which appends every chunk to `buffered_request_body_data_during_pause` with no bound, but the `self.pause_socket()` call that stops the kernel reads was under `#[cfg(not(windows))]` (`// TODO: figure out why windows is not emitting EOF with UV_DISCONNECT`). Every pause path (`req.pause()`, the `push() === false` -> `readStop(socket)` path from #34740, `req.socket.pause()`) ends in `do_pause`, so none of them reached TCP on Windows. ### Fix - Remove the cfg guard: `do_pause` calls `pause_socket()` on every platform (and `pause_socket` loses the `#[allow(dead_code)]` that existed only because it was dead on Windows). No other code changes. - Why the guard is obsolete: it was added in #18599 (March 2025), which taught the epoll and kqueue backends to still see a peer FIN/RST on a socket that is polling for nothing (`EPOLLRDHUP|EPOLLHUP|EPOLLERR`, a kept `EVFILT_WRITE`) but had no equivalent for the libuv backend. #32488 added that equivalent: `us_poll_start`/`us_poll_change` in `packages/bun-usockets/src/eventing/libuv.c` always arm `UV_DISCONNECT`, `poll_cb` probes a paused socket with `MSG_PEEK` to tell a graceful FIN (deferred until resume) from a reset (closed immediately), and the shared dispatch in `loop.c` defers EOF for a paused socket until it resumes. The symptom the TODO names is exactly what #32488 fixed. - Why a paused `node:http` socket always gets resumed: `do_resume` calls `resume_socket()` before any flag checks, and `end()`, `writeHeadAndEnd` and `abort()` resume the socket first as well, so a response ending with an unread body (`req._dump()` after `res.end()`) or a teardown re-arms the poll and any deferred FIN is delivered. This is the behavior Linux and macOS have had all along; this change gives Windows the same one. - Same primitive, already live on Windows: `Bun.serve` request-body backpressure (#36006, `RequestContext::pause_request_body_socket`) and the node:http pipelining flood guard (`pause_socket_reads`) call the same `uws_res_pause` -> `us_socket_pause` on every platform. - Verification: `test/js/node/http/node-http-backpressure.test.ts`, new `request body` group. Each stall test uploads a 32 MiB body into a request that is paused (explicitly, or implicitly by never being read) and requires the client's upload to stall short of the total, then resumes and requires all 32 MiB plus a 200 response; run over both http and https. A fifth test sends a small body plus FIN while the request is paused and requires them to be delivered on resume. - Windows x64, debug build without the fix: the 4 stall tests fail (`Expected: < 33554432, Received: 33554432`); with the fix the whole file passes (19/19). The FIN test passes on both and is coverage for the newly enabled deferred-EOF path, not the fail-before proof. - Linux, debug build: the file passes before and after (the compiled code is unchanged there), so the fail-before half of this proof exists only on Windows. - The standalone version of the stall scenario, same script under Node v26.3.0 and Bun on Linux: stalls at 2.75 MB; Bun on Windows with the fix: 3.25 MB (http), 3 MB (https); without the fix: all 32 MB sent, no stall. - Windows x64, all 498 upstream `test-http-*` / `test-https-*` files from `test/js/node/test/parallel` with the debug build: 497 pass both before and after. The one failure (`test-http-set-timeout-server.js`, a 1 ms `server.setTimeout` firing twice) is identical before and after and passes on the release canary. - Windows x64, `test/js/node/http` directory with the fix: 756 pass, 23 skip, 4 todo, 0 fail. ### Background - `us_socket_pause` drops the socket's readable interest in the event backend (epoll/kqueue on POSIX, libuv `uv_poll` on Windows); the kernel receive buffer then fills, the peer's send window closes, and its writes block. That is how read-side backpressure reaches a TCP peer. `us_socket_resume` re-adds the interest. - The pause contract in usockets: a FIN that arrives while a socket is paused is not acted on; it is re-discovered and delivered as `on_end` after the socket resumes. A reset closes the socket right away. The libuv backend needs extra machinery for this because Windows AFD only reports a FIN to a poll without read interest through the one-shot `UV_DISCONNECT` event; that machinery is what #32488 added. - `on_buffer_paused_shim` / `buffered_request_body_data_during_pause`: while a node:http request is paused, body chunks that uWS has already read are parked in this `Vec` and handed to JS as one `Buffer` on resume. With the socket actually paused it holds at most what was already in flight (one recv buffer); without the pause it held the rest of the upload. - Adjacent: #34761 (pipelined POST bodies) has context lines in this hunk but keeps the guard; it is a different bug. <details> <summary>Repro script and measurements</summary> ```js import http from "node:http"; import { once } from "node:events"; const got = Promise.withResolvers(); const server = http.createServer(req => { req.on("data", () => {}); req.pause(); got.resolve(req); }); await once(server.listen(0, "127.0.0.1"), "listening"); let pulls = 0; const CHUNK = 256 * 1024; const body = new ReadableStream({ pull(c) { pulls++; c.enqueue(new Uint8Array(CHUNK)); } }, { highWaterMark: 1 }); const ac = new AbortController(); fetch(`http://127.0.0.1:${server.address().port}/`, { method: "POST", body, duplex: "half", signal: ac.signal }).catch(() => {}); const req = await got.promise; for (let t = 500; t <= 2500; t += 500) { await new Promise(r => setTimeout(r, 500)); console.log({ ms: t, pulls, readableLength: req.readableLength, rssMB: Math.round(process.memoryUsage().rss / 1048576) }); } ac.abort(); req.destroy(); server.closeAllConnections(); server.close(); process.exit(0); ``` Windows x64, release canary `1.4.0-canary.1+9a543cc18` (unfixed): ``` {"ms":500,"pulls":1242,"readableLength":262144,"rssMB":368} {"ms":1000,"pulls":2527,"readableLength":262144,"rssMB":679} {"ms":1500,"pulls":4099,"readableLength":262144,"rssMB":2098} {"ms":2000,"pulls":6553,"readableLength":262144,"rssMB":1694} {"ms":2500,"pulls":8195,"readableLength":262144,"rssMB":2104} ``` Windows x64, debug build of this branch's parent (unfixed): ``` {"ms":500,"pulls":131,"readableLength":262144,"rssMB":215} {"ms":2500,"pulls":2051,"readableLength":262144,"rssMB":1652} ``` Windows x64, debug build with this change: ``` {"ms":500,"pulls":14,"readableLength":262144,"rssMB":101} {"ms":2500,"pulls":15,"readableLength":262144,"rssMB":101} ``` Linux, canary `da3851e57` (unchanged by this PR): `pulls` stays at 12, RSS 37 MB. </details>
2 participants
A
node:httpserver handler that never reads a POST body never sees'pause'onreq.connection. Node emits it once the body bytes pushed into the IncomingMessage reach its highWaterMark; code that keys its slow-reader flow on that event (including Node's owntest-http-no-read-no-dump) wedges forever on Bun.Repro
Node v26.3.0:
pause emitted, exit 0. Bun 1.4.0 / main:WEDGED: no pause event, exit 86.Cause
The native server dispatcher (
onNodeHTTPRequest) calledhandle.pause()on the body handle at dispatch and only installedhandle.ondatafromIncomingMessage.prototype._read(). So a handler that never read the body never hadonDataIncomingMessagerun at all: no bytes ever reached the Readable buffer,push()was never called, and nothing ever calledsocket.pause()on the connection.Node's
connectionListenerInternalkeeps the connection socket flowing throughsocketOnData-> parser ->parserOnBody, which pushes body bytes into the IncomingMessage unconditionally and callsreadStop(this.socket)(i.e.socket.pause(), which emits'pause') the momentpush()returnsfalse.IncomingMessage._read()laterreadStart()s the socket to resume.Bun's llhttp path (
_http_common.tsparserOnBody) already does this; only the native server path diverged.Fix
onNodeHTTPRequest: installhandle.ondata = onDataIncomingMessage.bind(http_req)at dispatch instead ofhandle.pause(), so body bytes flow into the IncomingMessage as they arrive (Node's eager-push model).onDataIncomingMessage: whenpush()returnsfalse, callreadStop(this.socket).NodeHTTPServerSocket.pause()forwards toresponse.pause()(so kernel reads still stop) andDuplex.pause()emits'pause'on the socket.NodeHTTPResponse::do_resume: once this request'sbody_read_statehas leftPending, skip re-registeringonData/onTimeoutwith uWS. Every uWS callback registration writes the per-socketHttpResponseData::userData, so asocket.resume()routed to the in-flight request's handle (viasocketHandle.response) after its body had completed was stealing a pipelined request'sonDatactx and dispatching that request's body fin (and trailers) to the previous request. The eager install above made that reachable on every pipelined chunked request; on main it was latent behind_read()'s lateset_on_datare-registering last.resume_socket()still runs so a deferred FIN on a paused fd is delivered._read()already callssocket.resume(), which restarts the native flow and drains anything buffered during the pause, so the existing resume path closes the cycle.As a consequence this also bounds
req.readableLengthwhen the handler reads slowly: theonDataIncomingMessagepath previously ignoredpush()'s return value entirely, so the Readable buffer grew unbounded (#26332). With thereadStopin place it stabilises at the highWaterMark, matching Node:req.readableLengthThis supersedes #30600, which backpressures via
handle.pause()directly. That bounds the buffer but still never emits'pause'onreq.socket(and never reaches its own check when the handler never reads, sinceondatawas only installed from_read()). Routing throughreadStop(socket)matches Node'sparserOnBodyexactly and fixes both symptoms.Verification
Two new tests in
node-http.test.ts:req.socket emits 'pause' once an unread request body fills the IncomingMessage buffer(the repro above)body reading from 'pause' still delivers every byte and 'end'(pause -> attach'data'-> drain 256 KB ->'end')Both time out on main and pass with this change, with output identical to Node v26.3.0. Node's upstream
test/parallel/test-http-no-read-no-dump.jsis added and now passes (hung forever before).The 376-file
test/js/node/test/parallel/test-http-*.jssweep has no new failures;node-http.test.ts,node-http-transfer-encoding.test.ts,node-http-backpressure.test.ts,node-http-connect.test.ts(11/12 vs 9/12 on main), express and body-parser integration tests are unchanged or improved.Fixes #26332.
[review] gate passed · iteration 10 · 7 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 7 passed · 1 rejected · iteration 10
evidence per changed file