node:http2: withhold connection window credit after setLocalWindowSize() decreases it
Fixes #43893
Stacked on #40180, which lands first. Review the last three commits.
Problem
setLocalWindowSize(0), thensetLocalWindowSize(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, then1 << 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 nextold - newbytes 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. Alsotest/js/node/http2/. Self-reviewed: 7 concerns, all addressed.
Background
- nghttp2 tracks
local_window_size(wanted),recv_window_size(used credit) andrecv_reduction(withheld credit). WINDOW_UPDATE only adds credit. H2FrameParserholds the engine in aRefCellthat 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-byteCelltakes, 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 thensetLocalWindowSize(1 << 20)):runtime increments state.localWindowSizenode 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, andstate.localWindowSizeis 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
connectionWindowSizeoption (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
localWindowSizestays 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 (
localWindowSize25535). 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_writeat once. AsetLocalWindowSize()call from there goes into the same queue, so the outer change applies first. The testa setLocalWindowSize() call from inside a transport writecovers this. With the queue applied late, it sends[10]where two calls in a row send[10, 65525]. -
set_local_window_sizerejects 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
statematches node. -
As in node,
state.localWindowSizeis 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_sizelet the engine write a WINDOW_UPDATE while it held the engine borrow. With two sessions in one process overduplexPair(), 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 testframes that arrive while setLocalWindowSize() writes its WINDOW_UPDATE are readtimes out on that version, and also when the drain is removed fromwith_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_readingthen finds the engine borrowed and leaves its stream WINDOW_UPDATE to the batch end, but this holder had none. A deferred call now setsengine_work_deferred, andwith_idle_engine()runsreplenish_windows()while that flag is set. The flag keeps_read()free of the scan over all streams. The testa stream resumed while setLocalWindowSize() writes its WINDOW_UPDATE gets its window backgetsstreamIncrements: []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 takesengine_work_deferred, and it runs again if its own writes deferred a call. Soreceive()andwith_idle_engine()get this through one function. The testa setLocalWindowSize() call from inside a batch-end write takes effect in that batchgetsincrements: []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 aduplexPair(), 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, soread()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 23setLocalWindowSize()tests pass in five runs, and the whole file passes (413 pass, 6 skip). -
rewrite_read()only applies queued window changes beforereceive()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_readinghad the same shape before this PR (replenish_streamwrites under the borrow), so it useswith_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 nestedsetLocalWindowSize()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::applycallsgrow(), which #41329 also calls for stream windows. This PR and #41329 conflict in one hunk ofh2_frame_parser.rs: #41329 moves the sync section ofrewrite_read()intosync_engine(). The PR that lands second moves theapply_recv_window_changes()call into that function. -
Rebased onto main after #43650. That PR made the h2 engine
pub(crate), and the workspace lintunreachable_pub = "deny"then rejected the plainpubitems here:RecvWindow::apply,RecvWindowChange,RecvWindowChange::increment,LocalWindow,LocalWindow::resizeandConnection::sync_recv_window. They arepub(crate)now. Struct fields staypub, 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_windowmirrors the window before it writes, sosession.stateis current inside the write. Aread()that re-enters inside the WINDOW_UPDATE write ofsetLocalWindowSize()takes the queued change beforereceive(), 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 ofrewrite_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 printsreceivedfor 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 testaccepts DATA that answers a setLocalWindowSize() call made inside a frame callbackis 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 temporaryconstassertion, linux-x64). Release builds on windows-x64,llvm-size -A:.text63735611 to 63736123 (+512),.rdata19938624 to 19938688 (+64),.reloc+4, 580 bytes in all. The file grows by 512 bytes (87818752 to 87819264). For each read batch the engine callstake_recv_window_change()twice (a 16-byteCelltake and a compare) andtake_deferred()twice (aCell<bool>swap). #40180 already has one such take beforereceive(). -
The tests for a smaller window sit in a nested
describeinside the block of #40180, and they use itsready(),frame()andframeReader()helpers. Each wait rejects when its session or stream emitserror, or emitsclosebefore the result is in. In a scratch test, a peer that drops the socket rejects the wait in 387 ms withclosed 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 hasof #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.tsfailed once in a GC count test (stream release after a queued END_STREAM, 5 live streams where the limit is 3). It does not callsetLocalWindowSize(). 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 intest/js/node/http2/, andtest-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.jsand 19 moretest-http2-*files.cargo clippy -p bun_runtimeis 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