mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
Send http2 window resize frames after half the window is used up (#25847)
### What does this PR do? It is standard practice to send HTTP2 window resize frames once half the window is used up: - [nghttp2](https://en.wikipedia.org/wiki/Nghttp2#HTTP/2_implementation) - [Go](https://github.com/golang/net/blob/master/http2/flow.go#L31) - [Rust h2](https://github.com/hyperium/h2/blob/master/src/proto/streams/flow_control.rs#L17) 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.
This commit is contained in:
@@ -1205,16 +1205,18 @@ pub const H2FrameParser = struct {
|
||||
var it = this.streams.valueIterator();
|
||||
while (it.next()) |stream| {
|
||||
log("incrementWindowSizeIfNeeded stream {} {} {} {}", .{ stream.id, stream.usedWindowSize, stream.windowSize, this.isServer });
|
||||
if (stream.usedWindowSize >= stream.windowSize) {
|
||||
if (stream.usedWindowSize >= stream.windowSize / 2 and stream.usedWindowSize > 0) {
|
||||
const consumed = stream.usedWindowSize;
|
||||
stream.usedWindowSize = 0;
|
||||
log("incrementWindowSizeIfNeeded stream {} {} {}", .{ stream.id, stream.windowSize, this.isServer });
|
||||
this.sendWindowUpdate(stream.id, UInt31WithReserved.init(@truncate(stream.windowSize), false));
|
||||
this.sendWindowUpdate(stream.id, UInt31WithReserved.init(@truncate(consumed), false));
|
||||
}
|
||||
}
|
||||
log("incrementWindowSizeIfNeeded connection {} {} {}", .{ this.usedWindowSize, this.windowSize, this.isServer });
|
||||
if (this.usedWindowSize >= this.windowSize) {
|
||||
if (this.usedWindowSize >= this.windowSize / 2 and this.usedWindowSize > 0) {
|
||||
const consumed = this.usedWindowSize;
|
||||
this.usedWindowSize = 0;
|
||||
this.sendWindowUpdate(0, UInt31WithReserved.init(@truncate(this.windowSize), false));
|
||||
this.sendWindowUpdate(0, UInt31WithReserved.init(@truncate(consumed), false));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user