Repository navigation
Conversation
lsquic picks the first send limit of a bidirectional stream by the role of the connection. RFC 9000 section 18.2 picks it by who opened the stream. The two agree only for a stream that the client opens. With unequal windows, a stream that a node:quic server opened sent into the wrong window in both directions. - patches/lsquic/stream-send-limit-by-initiator.patch keeps the two limits of the peer, under the names of the upstream draft litespeedtech/lsquic#629. An HTTP/3 connection keeps its one value. A unidirectional stream opened in 0-RTT keeps the unidirectional limit when the handshake completes. - A raw QUIC stream sends a FIN that has no data before it without a write event. lsquic gives a write event only to a stream that has flow-control credit, so such a FIN never left when the window of the stream or of the connection was 0.
|
Updated 8:38 AM PT - Oct 21st, 2026
❌ @robobun, your commit 36bb05a has 1 failures in
🧪 To try this PR locally: bunx bun-pr 46322That installs a local version of the PR into your bun-46322 --bun |
|
Status How the failures reproduce, on a build of #46321 (the base of this PR). Both ends are in one process, ALPN After
A stream that ends without data:
With this PR all three complete. The tests are in |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the IFC_HTTP block in the lsquic patch (it copies one limit over the other in the same direction the old role-keyed assignment chose, so HTTP/3 send limits are unchanged for both server and client) and the 0-RTT re-limit loop (a unidirectional stream now takes ifc_max_stream_data_uni, which matches what the resumed-session test pins).
Extended reasoning...
The change adds a vendored lsquic patch splitting the bidirectional send limit into local/remote per RFC 9000 18.2, and a Rust-side fast path that sends a bare FIN synchronously on non-HTTP/3 streams; the inline findings cover the FIN fast path, so this note only records what else in the patch was examined and ruled out.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
src/runtime/node/quic/stream.rs— A raw node:quic stream created with no body and destroyed in the same turn now reaches the peer as a reset stream where the base sent nothing. drain_outbound at stream.rs:526-528 now runs synchronously from kick_write for a FIN-only stream and sets wrote_to_lsquic, so abort_for_destroy at stream.rs:936 sends STOP_SENDING/RESET_STREAM at once instead of calling defer_stream_abort. The marker check in flush_deferred_aborts (session.rs:717-720), which drops the frames for Node parity when another stream wrote in between, is never reached. Fix: keep the created-and-abandoned path deferred for a stream whose only lsquic write was a bare FIN, or have the direct branch honor the same write_marker rule.Why this was flagged
Trigger: on a non-HTTP/3 session, JS calls stream.setBody(null) or createUnidirectionalStream({ body: new Uint8Array(0) }) and then stream.destroy() before the turn ends, with any write on another stream of the session in between. configureOutbound (quic.ts:1377-1379) calls endWrite -> end_write (stream.rs:792) -> kick_write (stream.rs:567); only_fin_pending is true, so drain_outbound runs, calls s.shutdown(1) and sets wrote_to_lsquic (stream.rs:527-528). destroy() then calls abortForDestroy (quic.ts:2085); abort_for_destroy sees wrote_to_lsquic true (stream.rs:936) and sends stop_sending/reset directly. On the base, wrote_to_lsquic was still false (the FIN waited for a write event), so defer_stream_abort ran and flush_deferred_aborts (session.rs:717-720) called shutdown_internal when the marker had moved, putting nothing on the wire. The code path that implements the stated Node-parity rule is now unreachable for FIN-only streams. Remedy: route the FIN-only case through the deferred-abort path, or apply the marker rule in the direct branch.
Verification: stream.rs:277-278 (bind_raw) and :568-569 (kick_write) now call drain_outbound() when only_fin_pending() holds; drain_outbound at :526-528 does s.shutdown(1); self.wrote_to_lsquic.set(true). abort_for_destroy at stream.rs:935-946: with the PR the first branch runs and frames go out at once; on the base the second branch ran and flush_deferred_aborts (session.rs:717-720) dropped them via shutdown_internal() when e.marker != marker.
| fn kick_write(&self) { | ||
| if let Some(s) = self.ls() { | ||
| if self.only_fin_pending() { | ||
| self.drain_outbound(); | ||
| } else if let Some(s) = self.ls() { | ||
| s.want_write(true); |
There was a problem hiding this comment.
🔴 A node:quic server that opens a bidirectional stream to a released bun client (1.4.x advertises initialMaxStreamDataBidiRemote 0) now sends nothing, with no blocked event and no error. With the stream window at 0, kick_write at src/runtime/node/quic/stream.rs:571 only sets want_write(true), and lsquic never delivers the write event, so drain_outbound never runs and note_write_blocked (the only StreamBlocked producer) never fires. Fix: when data is queued and the stream has no credit, surface the blocked state without a write event, so the application sees the stall the way node's onblocked does. The PR text calls this an accepted downside; in fact it is a silent loss for every default-configured 1.4.x peer, where the base delivered up to 16384 bytes. [also at: src/runtime/node/quic/stream.rs:572 - node:quic users whose peer advertises 0 for a stream (every released Bun <=1.4.2 does so by default for server-opened bidirectional streams) now get a silent stall: queued bytes never leave, no error, no onblocked, until idle timeout.; patches/lsquic/stream-send-limit-by-initiator.patch:42 - A node:quic app that opens a server-side bidirectional stream to a Bun 1.4.x peer now delivers 0 bytes of its body, with no error, where the base delivered it.]
Why this was flagged
A server built from this PR calls createBidirectionalStream({ body }) toward a bun 1.4.2 client, which by default advertises initial_max_stream_data_bidi_remote = 0. The patch at patches/lsquic/stream-send-limit-by-initiator.patch now gives that stream max_stream_send_bidi_remote = 0 (create_bidi_stream_out hunk). set_body queues the bytes and calls kick_write (src/runtime/node/quic/stream.rs:741); only_fin_pending is false because data is non-empty, so kick_write only calls want_write(true) at stream.rs:571. lsquic's stream_writeable requires write credit, so on_stream_write (stream.rs:1184) never runs, drain_outbound never runs, and note_write_blocked (stream.rs:480), the only source of SessionEvent::StreamBlocked, never fires. The user gets a stream whose body is never sent, no 'blocked' callback, no error, and sessions that end at the idle timeout with closed resolving cleanly. On the base branch the server took the client's bidi_local window instead and delivered up to 16384 bytes.
Verification: patches/lsquic/stream-send-limit-by-initiator.patch, create_bidi_stream_out hunk: max_stream_send_bidi_remote is now the send limit of a locally-opened bidi stream; on base the server used the peer's bidi_local. src/runtime/node/quic/stream.rs:567-572 kick_write with data queued only does s.want_write(true). drain_outbound is the only caller of note_write_blocked, the sole producer of SessionEvent::StreamBlocked (stream.rs:488).
| fn only_fin_pending(&self) -> bool { | ||
| let out = self.outbound.get(); | ||
| out.end == PendingEnd::Fin | ||
| && out.data.is_empty() | ||
| && !self.session_ref().is_some_and(QuicSession::is_http3) | ||
| } |
There was a problem hiding this comment.
🔴 Applications that set an empty or null body on a peer-opened unidirectional stream now lose the peer's data and see the stream close early. only_fin_pending() at stream.rs:560 is true for such a stream (JS setBody has no direction guard), so kick_write calls drain_outbound, which after the FIN runs s.shutdown(0) on the read side (stream.rs:531-533). Fix: skip the direct-FIN path whenever the write side is already ended (write_ended != 0, set for inbound uni streams at stream.rs:271), or make set_body/end_write ignore a FIN on a receive-only stream like the JS writer getter does. On the base want_write(true) is refused by lsquic for that stream, so nothing was shut down.
Why this was flagged
A raw (non-HTTP/3) session; the peer opens a unidirectional stream and the application's onstream handler calls stream.setBody(null) (quic.ts:1377-1380 -> handle.endWrite()). quic.ts:2373-2390 checks only outboundSet/writer, not direction. end_write (stream.rs:797-808) sets o.end = PendingEnd::Fin with empty data and calls kick_write. only_fin_pending() (stream.rs:560-565) is true because the session is not HTTP/3, so kick_write (stream.rs:568-569) calls drain_outbound directly. There s.shutdown(1) runs (stream.rs:527), and because the id has STREAM_ID_UNI_BIT it calls s.shutdown(0) (stream.rs:531-533), shutting the read side of the peer's stream. Later bytes and the peer's FIN never reach JS. On the base branch kick_write only called s.want_write(true); lsquic refuses lsquic_stream_wantwrite on a stream whose write side is done (bind_raw marks write_ended = 1 at stream.rs:271), so drain_outbound never ran. The new direct path bypasses that lsquic check and nothing in only_fin_pending looks at write_ended.
Verification: pre-existing — the base branch reaches the same read-side shutdown by the same route one lsquic tick later, so this PR does not introduce the data loss. Trigger: a raw session whose onstream handler calls setBody(null) on a peer-opened unidirectional stream. On the base, kick_write called s.want_write(true) and on_stream_write (stream.rs:1184-1186) calls the same unchanged drain_outbound (stream.rs:525-533).
|
A second branch for this defect exists: Three differences that can be of use here:
Other numbers from the branch, linux x64, release builds of main 9634520 and the branch:
|
|
#46446 has the rule for a stream that ends with nothing queued, as its own change. It is meant to land before #46321 and this PR. It differs from
On the branch of #46446, without the lsquic patch of this PR, these tests of this PR pass: the 8 tests of "a stream that ends without data" and the 4 tests of "an HTTP/3 stream whose body ends before its headers are sent". With #46446 under this PR, these parts are not needed here:
|
Problem
apply_trans_params,lsquic_full_conn_ietf.c:3671). RFC 9000 section 18.2 picks it by who opened the stream.flow control violation on stream 1, or stalls.Fix
patches/lsquic/stream-send-limit-by-initiator.patchkeeps both limits of the peer, named as in upstream's draft WebTransport and HTTP Datagram Support litespeedtech/lsquic#629. HTTP/3 connections keep their one value. A unidirectional stream opened in 0-RTT keeps the unidirectional limit.stream.rssends a raw QUIC stream's FIN at once when no data is queued.quic-stream.test.ts, 21 new tests, 14 fail without this PR. Alsotest-quic-*.mjs,serve-http3.test.ts,fetch-http3-client.test.ts.Background
initial_max_stream_data_bidi_remotelimits a stream this side opens. Itsbidi_locallimits a stream the peer opens.Downsides
Bun.serveandfetchincluded. mimalloc's block stays 10240 bytes.Notes
Stack. This PR is based on #46321. That one repairs the default case with no change to lsquic. This one makes the sender correct for any pair of windows. The diff here is only this PR.
How it was found. The failure is in the "Found, not in this PR" list of #46054. No user reported it.
The lsquic patch, hunk by hunk (
lsquic_full_conn_ietf.c, 6 hunks, 105 lines).struct ifc_cfg:max_stream_sendbecomesmax_stream_send_bidi_localandmax_stream_send_bidi_remote, the names of WebTransport and HTTP Datagram Support litespeedtech/lsquic#629. Upstream master (a9616720, v4.10.2) still has the one role-keyed limit. The vendored version is v4.9.4.apply_trans_params: both limits are stored from the peer's transport parameters. The role-keyed assignment is removed.create_bidi_stream_outreadsbidi_remote.new_streamreadsbidi_local.initialMaxStreamDataBidiRemotewas below the bytes already sent.IFC_HTTP): the limit for server-opened streams keeps the value of the client-opened ones. This part is not in Create share image for social media #629. lsquic's transport parameter check bounds only that value from below (4096). With a limit of 0, the stream that an HTTP/3 node:quic server is not allowed to open reachesassert(size <= stream_avail)instream_write_avail_with_frames(reproduced under gdb). A bump to an lsquic with Create share image for social media #629 has to replace this block, not only drop it.The FIN change.
drain_outboundsends the FIN, and it ran only from lsquic's write event. lsquic gives that event only when the stream and the connection have credit (stream_writeable).kick_writeandbind_rawnow calldrain_outboundat once when only the FIN is queued. HTTP/3 sessions are excluded:createBidirectionalStream({ headers, body })queues the end of an empty body before the header block, and a FIN before the headers makes lsquic reset the stream. Four tests pin that order. 18 HTTP/3 orderings (end before or aftersendHeaders, from a microtask or a timer) and 24 create-then-abandon sequences give the same result as a build of #46321.Measurements. Release builds of #46321 and of this PR.
struct ietf_full_conn:calloc(1, 10232)becomescalloc(1, 10240)inlsquic_ietf_full_conn_client_newand_server_new(objdump).mi_usable_sizeis 10240 for both sizes and 12288 for 10248 (gdb).create_bidi_stream_out: 98 instructions and 6 conditional jumps before and after.new_stream: 189 and 19 before and after. Only load displacements differ.apply_trans_params(once per connection): 340 to 347 instructions, 44 to 47 conditional jumps, +32 bytes.QuicStream::bind_raw: 294 to 307 instructions.kick_writeis inlined in 7 host functions: +13 to +16 instructions each. Awrite()with data queued executes 2 more.Bun.servewithfetch) and 0 of 46 (node:quic h3) firstmax_send_offvalues differ (BUN_DEBUG_lsquic=1, debug builds).size), 10 symbols changed.Mixed versions. Two processes on loopback, a server-opened stream, 6 runs each. "1.4.2" is the released bun, which advertises a window of 0 for such a stream and accepts 16384 bytes.
In the two "0 bytes" rows nothing reports an error. The body is never sent, the sessions end at the idle timeout, and the
closedpromises resolve. Workaround on the 1.4.x end: setinitialMaxStreamDataBidiRemote(client) orinitialMaxStreamDataBidiLocal(server). With 1048576 there, both pairings carry 262144 bytes into that window and 1000 bytes the other way in 6 of 6 runs.Tests. Of the 21 new tests, 14 fail on a build of #46321. The other 7 guard one part each:
test-quic-zero-rtt.mjs.The tests that end a stream without data turn the idle timeout off. With it, a session whose FIN never leaves ends after 10 s and both readers see the same clean end as after a FIN.
Also run on the debug build: all 235
test-quic-*.mjsfiles (the same 3 fail as on main: 2 importnode:stream/iter, 1 asserts an RTT below 10 ms;test-quic-callback-error-*fail in 1 to 2 of 6 runs on both builds),quic-endpoint.test.tsandquic-sni.test.ts(18 pass),serve-http3.test.ts(79 pass),fetch-http3-client.test.ts(67 pass).Self-review, not taken.
session.opened, and the server's 60 s no-progress timer: other defects, not touched here.Order with other work. The branch
robobun/3fc05d46/quic-stream-credit-on-pullhas the same 0-RTT hunk and a work-around for this bug. The second of the two to land drops its copy. #37491 makesOutbound.dataprivate.only_fin_pendingthen readsout.is_empty().