node:http2: withhold connection window credit after setLocalWindowSize() decreases it

RO
1mo ago
+1182-97

Fixes #43893

Stacked on #40180, which lands first. Review the last three commits.

Problem

  • setLocalWindowSize(0), then setLocalWindowSize(2**31-1), ends the session. Bun adds +2147483647 to the peer's 65535: FLOW_CONTROL_ERROR (RFC 9113 §6.9.1).
  • H2FrameParser::set_local_window_size (src/runtime/api/bun/h2_frame_parser.rs) ignores a decrease. For 20, then 1 << 20, bun sends +1048556 where node sends +983041.
  • #43893: the call inside a frame callback, on a synchronous transport, ends the session.

Fix

  • A decrease works like nghttp2's recv_reduction, as in node. The next old - new bytes earn no WINDOW_UPDATE, and a raise repays them first.
  • The binding keeps the size and the reduction (LocalWindow). The engine takes queued changes before each read and WINDOW_UPDATE. That fixes #43893.
  • with_idle_engine() runs what JS deferred during a WINDOW_UPDATE write: a session read, or a resumed stream.
  • Verified: fifteen tests in test/js/node/http2/node-http2.test.js, fourteen fail on main and on #40180. Also test/js/node/http2/. Self-reviewed: 7 concerns, all addressed.

Background

  • nghttp2 tracks local_window_size (wanted), recv_window_size (used credit) and recv_reduction (withheld credit). WINDOW_UPDATE only adds credit.
  • H2FrameParser holds the engine in a RefCell that a read borrows. JS calls during a read are queued.
  • Considered an engine-only reduction: JS can call before the engine exists. Considered no write under the borrow: nested frames hit #43538.

Downsides

  • Behavior change: a lower window now takes effect. After setLocalWindowSize(20), the peer gets 20 bytes for each round trip once its 65535 are used.
  • Every session: +16 bytes (H2FrameParser, now 1448). Each read batch: two 16-byte Cell takes, two flag swaps, no allocation. Release binary: +580 bytes (windows-x64).
Notes
  • Reproduction: the script from the report (a raw server that logs connection WINDOW_UPDATE increments, and a client that calls setLocalWindowSize(20) and then setLocalWindowSize(1 << 20)):

    runtimeincrementsstate.localWindowSize
    node v26.3.0[983041]1048576
    bun 1.4.1 (main)[1048556]1048576
    #40180 alone[1048556]1114091
    this PR[983041]1048576
  • The teardown, with a bun client and a bun server in one process: bun 1.4.1 gets GOAWAY with code 3 (FLOW_CONTROL_ERROR) and ERR_HTTP2_SESSION_ERROR. Node and this PR get the response, and state.localWindowSize is 2147483645 in both.

  • Who calls this with a smaller value: on bun, the known callers raise the window. grpc-js calls it only for a value above 65535. undici uses its connectionWindowSize option (default 512 KiB). http2-wrapper asks for 4 MiB. After nodejs/node#64623, the default connection window of node is 32 MiB, so that http2-wrapper call is a decrease on node. Bun still starts at 65535.

  • setLocalWindowSize() takes one of three paths. The engine is not created yet (before the first read), the engine is borrowed (a frame callback), or the engine is free (between reads). A temporary log confirmed that the three variants of the first test cover one path each.

  • A raise that the withheld credit covers sends nothing while the peer has window left. For 20, then 30000, node and this PR send no WINDOW_UPDATE, and localWindowSize stays 65535. Main sends +29980.

  • A raise after the peer used the withheld credit: the peer sends its 65535 bytes after a decrease to 0, and the raise comes between reads. For a raise to 100000, node and this PR send +34465 and then +65535 at once. For a raise to 65535, node sends nothing, and its transfer stalls. This PR sends +65535. The first version of this PR waited for the next inbound frame.

  • The last test covers a decrease in a PING callback, with 40000 bytes of DATA after the PING in the same read. Node withholds all 40000 bytes (localWindowSize 25535). Without the engine-side take, the WINDOW_UPDATE check at the end of the read used the old size and granted 40000.

  • A write can run JS: cork() flushes the output of another session, and a JS transport runs its _write at once. A setLocalWindowSize() call from there goes into the same queue, so the outer change applies first. The test a setLocalWindowSize() call from inside a transport write covers this. With the queue applied late, it sends [10] where two calls in a row send [10, 65525].

  • set_local_window_size rejects a size outside 0..=2^31-1. The JS layer checks the range first.

  • Interop: a 70000 byte upload and a 70000 byte response, with both windows lowered to 20. All four pairs of bun and node finish, and the final state matches node.

  • As in node, state.localWindowSize is the window that the peer still has, so a decrease alone does not lower it.

  • A review found a hang in an earlier version of this PR. set_local_window_size let the engine write a WINDOW_UPDATE while it held the engine borrow. With two sessions in one process over duplexPair(), that write flushed the other session's corked PING into this session. read() found the engine borrowed and only queued the bytes, and nothing read them until the next frame came in. main does not hang there, because it writes before it borrows. The test frames that arrive while setLocalWindowSize() writes its WINDOW_UPDATE are read times out on that version, and also when the drain is removed from with_idle_engine(). It fails on main and on #40180.

  • A second review finding, same cause: JS inside that write can resume a paused stream of the same session. set_stream_reading then finds the engine borrowed and leaves its stream WINDOW_UPDATE to the batch end, but this holder had none. A deferred call now sets engine_work_deferred, and with_idle_engine() runs replenish_windows() while that flag is set. The flag keeps _read() free of the scan over all streams. The test a stream resumed while setLocalWindowSize() writes its WINDOW_UPDATE gets its window back gets streamIncrements: [] without the fix. It needs large windows: under default windows the peer can send 65535 bytes, one less than the 64 KiB that a stream buffers, so the stream never pauses natively.

  • A third review finding, same cause: JS inside a WINDOW_UPDATE write of the batch end itself can call setLocalWindowSize() or resume a stream. That call came too late for the pass that was running. replenish_windows() now loops: each pass first takes engine_work_deferred, and it runs again if its own writes deferred a call. So receive() and with_idle_engine() get this through one function. The test a setLocalWindowSize() call from inside a batch-end write takes effect in that batch gets increments: [] without the fix. It passes on main and on #40180 alone, where the batch end sends that update itself, because nothing is withheld. A session on a native socket cannot reach this case: bun flushes every cork when a JS callback returns to native code. The test runs the session on a duplexPair(), where the read runs inside a JS call.

  • The resumed-stream test failed on both Windows lanes of build 120069. That was a flaw in the test. read() returned only the first buffered chunk, and on Windows that chunk was small, so read() never reached _read(). The test now reads the whole buffer, and it sizes the windows (stream 200000, connection 150000, 120000 bytes of DATA) so that the stream pauses before its update comes due, however the transport splits the reads. On a Windows machine, all 23 setLocalWindowSize() tests pass in five runs, and the whole file passes (413 pass, 6 skip).

  • rewrite_read() only applies queued window changes before receive() and writes nothing there. A write at that point could put a re-entrant read ahead of the bytes of the current read.

  • set_stream_reading had the same shape before this PR (replenish_stream writes under the borrow), so it uses with_idle_engine() too. That site has no failing test: in my attempt the deferred stream WINDOW_UPDATE went out before the other session had a corked frame. The helper changes nothing there unless bytes were queued during the call.

  • Not fixed here, older than this PR: cork() resets its offset after it takes the cork from another session, so a frame that the flush's JS corked for this session is lost (#43538). A nested setLocalWindowSize() raise with a positive increment loses its WINDOW_UPDATE that way, on main too. The engine path of this PR avoids it: a nested change stays queued until the outer write returns.

  • RecvWindow::apply calls grow(), which #41329 also calls for stream windows. This PR and #41329 conflict in one hunk of h2_frame_parser.rs: #41329 moves the sync section of rewrite_read() into sync_engine(). The PR that lands second moves the apply_recv_window_changes() call into that function.

  • Rebased onto main after #43650. That PR made the h2 engine pub(crate), and the workspace lint unreachable_pub = "deny" then rejected the plain pub items here: RecvWindow::apply, RecvWindowChange, RecvWindowChange::increment, LocalWindow, LocalWindow::resize and Connection::sync_recv_window. They are pub(crate) now. Struct fields stay pub, as on main. The earlier commits of this PR are one commit now.

  • Rebased onto the head of #40180 (b21031b3b5) on 2026-09-24. Its six commits come first on this branch, unchanged. #40180 lands first, and its Notes name this PR as the owner of the accounting after a decrease. That head pins two orders with tests, and this PR keeps both. replenish_connection_window mirrors the window before it writes, so session.state is current inside the write. A read() that re-enters inside the WINDOW_UPDATE write of setLocalWindowSize() takes the queued change before receive(), so the engine checks that DATA against the new window. With each of the two lines reverted, exactly the matching #40180 test fails.

  • #43893: setLocalWindowSize() inside a frame callback, on a transport that delivers the peer's answer synchronously. On #40180 the read that re-enters is queued, and the drain loop of rewrite_read() fed it to the engine before the window growth was applied, so a body of 65536 bytes ended the session with FLOW_CONTROL_ERROR. Here the engine takes queued changes at the end of each batch, which is before that drain. The script from the issue prints received for 65535, 65536 and 100000 bytes on this branch, and an error for the last two on #40180. Node aborts on that transport with its own assertion, so there is no node result. The test accepts DATA that answers a setLocalWindowSize() call made inside a frame callback is that script.

  • No user reported either case.

  • Measurements for Downsides, against the head of #40180. size_of::<H2FrameParser>() is 1432 bytes there and 1448 bytes here (read from a temporary const assertion, linux-x64). Release builds on windows-x64, llvm-size -A: .text 63735611 to 63736123 (+512), .rdata 19938624 to 19938688 (+64), .reloc +4, 580 bytes in all. The file grows by 512 bytes (87818752 to 87819264). For each read batch the engine calls take_recv_window_change() twice (a 16-byte Cell take and a compare) and take_deferred() twice (a Cell<bool> swap). #40180 already has one such take before receive().

  • The tests for a smaller window sit in a nested describe inside the block of #40180, and they use its ready(), frame() and frameReader() helpers. Each wait rejects when its session or stream emits error, or emits close before the result is in. In a scratch test, a peer that drops the socket rejects the wait in 387 ms with closed before the test had its result. The old wiring waited for the test timeout there.

  • With the decrease in effect, the test a smaller size keeps the window the peer already has of #40180 got slow. After the first 65535 bytes its peer gets 20 bytes for each round trip, so the 100000 byte body took 1724 round trips (1548 ms on the debug build). The body is now 65535 + 200 bytes: ten round trips, 157 ms. The test still passes on #40180 alone.

  • h2-conformance.test.ts failed once in a GC count test (stream release after a queued END_STREAM, 5 live streams where the limit is 3). It does not call setLocalWindowSize(). It passed in three filtered runs and in two runs of the whole file.

  • Also run with the debug build: node-http2.test.js, the other files in test/js/node/http2/, and test-http2-client-setLocalWindowSize.js, test-http2-server-setLocalWindowSize.js, test-http2-window-size.js, test-http2-session-stream-state.js, test-http2-misbehaving-flow-control.js, test-http2-window-update-overflow.js and 19 more test-http2-* files. cargo clippy -p bun_runtime is clean.


[human-review] gate passed · iteration 0 · 4 files touched

fails on main (without fix)
ASAN without fix: 21 failed, 6 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/node/http2/node-http2.test.js"
bun test v1.4.3 (367d939d9)

test/js/node/http2/node-http2.test.js:
(pass) node none > Client Basics > should be able to send a GET request [1015.61ms]
(pass) node none > Client Basics > should be able to send a POST request [755.28ms]
(pass) node none > Client Basics > constants [27.98ms]
(pass) node none > Client Basics > getDefaultSettings [3.35ms]
(pass) node none > Client Basics > getPackedSettings/getUnpackedSettings [25.60ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is too small [7.81ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a multiple of 6 bytes [4.42ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a buffer [7.41ms]
(pass) node none > Client Basics > should be able to send data using end [803.42ms]
(pass) node none > Client Basics > should be able to mutiplex GET requests [781.26ms]
(pass) node none > Client Basics > http2 should receive remoteSettings when receiving 
... (truncated)

release without fix: 23 failed, 6 skipped
bun test v1.4.3-canary.1 (367d939d9)

test/js/node/http2/node-http2.test.js:
(pass) node none > Client Basics > constants [0.83ms]
(pass) node none > Client Basics > getDefaultSettings [0.04ms]
(pass) node none > Client Basics > getPackedSettings/getUnpackedSettings [0.26ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is too small [0.13ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a multiple of 6 bytes [0.04ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a buffer [0.06ms]
(pass) node none > Client Basics > is possible to abort request [1.62ms]
(pass) node none > Client Basics > aborted event should work with abortController [0.93ms]
(pass) node none > Client Basics > aborted event should work with aborted signal [0.88ms]
(pass) node none > Client Basics > signal validation matches node: non-signal objects throw, duck-typed { aborted } is accepted [0.90ms]
(pass) node none > Client Basics > should fail to connect over HTTP/1.1 [36.24ms]
(skip) node none > Client Basics > should not leak memory
(pass) node none > Client Basics > headers cannot be bigge
... (truncated)
passes on PR (with fix)
ASAN with fix: 6 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/node/http2/node-http2.test.js"
bun test v1.4.3 (367d939d9)

test/js/node/http2/node-http2.test.js:
(pass) node none > Client Basics > should be able to send a GET request [1100.50ms]
(pass) node none > Client Basics > should be able to send a POST request [777.29ms]
(pass) node none > Client Basics > constants [29.63ms]
(pass) node none > Client Basics > getDefaultSettings [3.14ms]
(pass) node none > Client Basics > getPackedSettings/getUnpackedSettings [26.36ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is too small [7.75ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a multiple of 6 bytes [4.39ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a buffer [7.53ms]
(pass) node none > Client Basics > should be able to send data using end [817.67ms]
(pass) node none > Client Basics > should be able to mutiplex GET requests [803.42ms]
(pass) node none > Client Basics > http2 should receive remoteSettings when receiving 
... (truncated)

release with fix: 6 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 1128ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/224] gen generated_host_exports.rs
generated_host_exports.rs: 121 exports (host=5, lazy=10, generic=106, rust=0); 245 extern-C blocks audited
[2/224] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (15 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts
  - ResourceUsage (8 fields)
  - Subprocess (20 fields)
Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts
  - CronJob (5 fields)
Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts
  - FileSystemRouter (5 fields)
  - FrameworkFileSystemRouter (2 fields)
  - MatchedRoute (8 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Glob.classes.ts
  - Glob (5 fields)
Found 1 classes from /workspace/bun/src/runtime/api/h2.classes.ts
  - H2FrameParser (29 fields)
Found
... (truncated)
diff hotspot
src/runtime/api/bun/h2/connection.rs   |  98 +++-
 src/runtime/api/bun/h2/flow_control.rs | 110 +++-
 src/runtime/api/bun/h2_frame_parser.rs | 154 +++---
 test/js/node/http2/node-http2.test.js  | 917 +++++++++++++++++++++++++++++++++
 4 files changed, 1182 insertions(+), 97 deletions(-)

gate history · 5 passed · 0 rejected · iteration 0

evidence per changed file
file                                    reads  edits  tests
src/runtime/api/bun/h2/connection.rs       11     17    157
src/runtime/api/bun/h2/flow_control.rs      6     12    156
src/runtime/api/bun/h2_frame_parser.rs     29     33    156
test/js/node/http2/node-http2.test.js      21     23    156