Send http2 window resize frames after half the window is used up by jackkleeman · Pull Request #25847 · oven-sh/bun

jackkleeman · 2026-01-06T10:03:28Z

What does this PR do?

It is standard practice to send HTTP2 window resize frames once half the window is used up:

The current behaviour of Bun is to send a window update once the client has sent 65535 bytes exactly. This leads to an interruption in throughput while the window update is received.

This is not just a performance concern however, I think some clients will not handle this well, as it means that if you stop sending data even 1 byte before 65535, you have a deadlock. The reason I came across this issue is that it appears that the Rust hyper crate always reserves an additional 1 byte from the connection for each http2 stream (https://github.com/hyperium/hyper/blob/master/src/proto/h2/mod.rs#L122). This means that when you have two concurrent requests, the client treats it like the window is exhausted when it actually has a byte remaining, leading to a sequential dependency between the requests that can create deadlocks if they depend on running concurrently. I accept this is not a problem with bun, but its a happy accident that we can resolve such off-by-one issues by increasing the window size once it is 50% utilized

How did you verify your code works?

Using wireshark, bun debug logging, and client logging I can observe that the window updates are now sent after 32767 bytes. This also resolves the h2 crate client issue.

coderabbitai · 2026-01-06T10:05:26Z

Walkthrough

HTTP/2 flow-control changed: WINDOW_UPDATE is now sent when consumed bytes reach at least half the window (and >0); the update amount equals the consumed bytes, which are then reset. Applies to both stream-level and connection-level flow control.

Changes

Cohort / File(s)	Change Summary
HTTP/2 Flow Control Thresholds `src/bun.js/api/bun/h2_frame_parser.zig`	Trigger condition changed from `usedWindowSize >= windowSize` to `usedWindowSize >= windowSize / 2` and `usedWindowSize > 0`. When triggered, capture `consumed = usedWindowSize`, reset `usedWindowSize` to `0`, and send `WINDOW_UPDATE` with `consumed` (instead of the full window size). Applies to both per-stream and connection-level updates.

Pre-merge checks

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: sending HTTP/2 window resize frames after half the window is used (32,767 bytes instead of 65,535 bytes).
Description check	✅ Passed	The description covers both required template sections: 'What does this PR do?' with detailed context and standards references, and 'How did you verify your code works?' with verification methodology.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 683838a1e6af443ce5e2de2c12565ada936cf723 and 90ed2e8.

📒 Files selected for processing (1)

src/bun.js/api/bun/h2_frame_parser.zig

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.zig

📄 CodeRabbit inference engine (src/CLAUDE.md)

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Files:

src/bun.js/api/bun/h2_frame_parser.zig

**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, be careful with allocators and use defer for cleanup

Files:

src/bun.js/api/bun/h2_frame_parser.zig

🧠 Learnings (5)

📚 Learning: 2025-10-17T20:50:58.644Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-10-19T03:01:29.084Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-10-18T20:59:45.579Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-11-10T00:57:09.173Z

Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2026-01-05T16:32:07.551Z

Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI Agents

In @src/bun.js/api/bun/h2_frame_parser.zig:
- Around line 1216-1219: The connection-level WINDOW_UPDATE logic can send a
zero-increment if this.windowSize is 0 or 1; update the guard in the block that
checks this.usedWindowSize >= this.windowSize / 2 (around the sendWindowUpdate
call) to also require this.usedWindowSize > 0 so you only call
sendWindowUpdate(0, UInt31WithReserved.init(@truncate(consumed), false)) when
consumed is positive; adjust the condition and keep references to
usedWindowSize, windowSize, sendWindowUpdate, and UInt31WithReserved.init to
locate and fix the code.
- Around line 1208-1212: incrementWindowSizeIfNeeded can send a WINDOW_UPDATE
with a zero increment when stream.windowSize is 0 or 1 because the check uses
stream.usedWindowSize >= stream.windowSize / 2; change the guard in
incrementWindowSizeIfNeeded to also require stream.usedWindowSize > 0 (i.e.,
only send a window update when usedWindowSize is non-zero) so sendWindowUpdate
is never called with a zero increment; also consider adding a minimum validation
in setLocalWindowSize to prevent extremely small windowSize values if desired.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0a8e57e24f43727ab356f3989ca6a330fe2e7ebf and 683838a1e6af443ce5e2de2c12565ada936cf723.

📒 Files selected for processing (1)

src/bun.js/api/bun/h2_frame_parser.zig

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.zig

📄 CodeRabbit inference engine (src/CLAUDE.md)

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Files:

src/bun.js/api/bun/h2_frame_parser.zig

**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, be careful with allocators and use defer for cleanup

Files:

src/bun.js/api/bun/h2_frame_parser.zig

🧠 Learnings (4)

📚 Learning: 2025-10-17T20:50:58.644Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-10-18T04:46:33.434Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:80-156
Timestamp: 2025-10-18T04:46:33.434Z
Learning: In src/bun.js/telemetry.zig, the Bun.telemetry.configure() API is designed for a single telemetry adapter use case (primarily bun-otel). The merge semantics (where omitted callbacks are left intact) is intentional. If multiplexing multiple telemetry consumers is needed, it should be implemented in userspace by overriding the BunSDK class, not in the core API.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-10-19T03:01:29.084Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2026-01-05T16:32:07.551Z

Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

🔇 Additional comments (1)

src/bun.js/api/bun/h2_frame_parser.zig (1)

1208-1219: LGTM with concerns flagged above.

The core change to send WINDOW_UPDATE at 50% consumption (instead of 100%) is correct and aligns with nghttp2, Go, and Rust h2 implementations as noted in the PR objectives. Using the accumulated consumed bytes and resetting usedWindowSize is the proper approach. This should resolve the throughput interruption and off-by-one deadlock issues mentioned in the PR.

The major concerns flagged above regarding zero-increment frames are edge cases that would only occur with misconfigured window sizes.

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/bun.js/api/bun/h2_frame_parser.zig (1)

1204-1221: Add MAX_WINDOW_SIZE validation to setLocalWindowSize for RFC 7540 compliance.

The setLocalWindowSize function accepts window size values without validating against MAX_WINDOW_SIZE (2^31-1), unlike the initialWindowSize validation at line 2710 which correctly checks if (initialWindowSizeValue > MAX_WINDOW_SIZE or initialWindowSizeValue < 0). Without this guard, windowSize can be set to values exceeding 2^31-1, allowing usedWindowSize to grow beyond the RFC 7540 limit and causing data loss in the subsequent @truncate(consumed) operations at lines 1212 and 1219 in incrementWindowSizeIfNeeded.

Apply the same MAX_WINDOW_SIZE bounds check in setLocalWindowSize before assigning windowSizeValue to this.windowSize.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 683838a1e6af443ce5e2de2c12565ada936cf723 and 90ed2e8.

📒 Files selected for processing (1)

src/bun.js/api/bun/h2_frame_parser.zig

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.zig

📄 CodeRabbit inference engine (src/CLAUDE.md)

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Files:

src/bun.js/api/bun/h2_frame_parser.zig

**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, be careful with allocators and use defer for cleanup

Files:

src/bun.js/api/bun/h2_frame_parser.zig

🧠 Learnings (5)

📚 Learning: 2025-10-17T20:50:58.644Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-10-19T03:01:29.084Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-10-18T20:59:45.579Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2025-11-10T00:57:09.173Z

Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

📚 Learning: 2026-01-05T16:32:07.551Z

Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.

Applied to files:

src/bun.js/api/bun/h2_frame_parser.zig

jackkleeman mentioned this pull request Jan 6, 2026

H2 streams reserve 1 more byte than needed from the connection, leading to deadlock hyperium/hyper#4003

Closed

Jarred-Sumner requested a review from cirospaciari January 6, 2026 12:59

jackkleeman force-pushed the window-resize-half branch from 0a8e57e to 683838a Compare January 6, 2026 15:44

coderabbitai Bot reviewed Jan 6, 2026

View reviewed changes

Comment thread src/bun.js/api/bun/h2_frame_parser.zig Outdated

Comment thread src/bun.js/api/bun/h2_frame_parser.zig Outdated

Send http2 window resize frames after half the window is used up

90ed2e8

jackkleeman force-pushed the window-resize-half branch from 683838a to 90ed2e8 Compare January 6, 2026 16:03

coderabbitai Bot reviewed Jan 6, 2026

View reviewed changes

cirospaciari approved these changes Jan 6, 2026

View reviewed changes

cirospaciari merged commit 46801ec into oven-sh:main Jan 6, 2026
47 of 52 checks passed

jackkleeman deleted the window-resize-half branch January 6, 2026 19:40

coderabbitai Bot mentioned this pull request Jan 18, 2026

fix(http2): fix settings, window size handling, and dynamic header buffer allocation #26119

Merged

4 tasks

sgbalogh mentioned this pull request Feb 11, 2026

node:http2 client ignores initialWindowSize setting and never sends WINDOW_UPDATE frames — streams stall at 65,535 bytes #26915

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send http2 window resize frames after half the window is used up#25847

Send http2 window resize frames after half the window is used up#25847
cirospaciari merged 1 commit into
oven-sh:mainfrom
jackkleeman:window-resize-half

jackkleeman commented Jan 6, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jan 6, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jackkleeman commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

How did you verify your code works?

Uh oh!

coderabbitai Bot commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Pre-merge checks

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jackkleeman commented Jan 6, 2026 •

edited

Loading

coderabbitai Bot commented Jan 6, 2026 •

edited

Loading