Skip to content

net.http: track h2 server connections for idle shutdown once, not per frame #27435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JalonSolov merged 1 commit into from
Jun 15, 2026
+20 −29
Merged

net.http: track h2 server connections for idle shutdown once, not per frame #27435

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 20 additions & 29 deletions vlib/net/http/h2_server.v
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,28 @@ fn serve_h2_conn_with_idle_tracker(mut transport H2Transport, mut handler Handle
}

fn (mut c H2ServerConn) serve(mut handler Handler) ! {
c.read_client_preface_idle()!
// Register the connection with the idle tracker once for its whole
// lifetime, instead of around every frame read. The reader thread spends
// nearly all of its time blocked in a frame read, so per-frame mark/unmark
// only added shared-lock contention (and an O(n) list scan) on the hot
// path. On shutdown, close_idle still interrupts the reader by shutting the
// fd down; an h2 request in flight when the server stops is interrupted,
// which is acceptable at shutdown and is not relied on by any caller (the
// graceful "wait for active request" guarantee is HTTP/1.1-only).
tracked := c.should_track_idle_read()
if tracked && !c.idle_conns.mark_idle(c.idle_handle) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep active H2 requests out of the idle tracker

When a client negotiates HTTP/2 and Server.close() runs while handler.handle() is still processing a request, this lifetime mark_idle leaves the active connection in idle_conns, so listen_and_serve_tls calls idle_conns.close_idle() and shuts the fd before the handler can send its response. Previously read_idle_frame() unmarked before dispatch_frame()/run_request(), matching the active-request close behavior covered by test_server_tls_close_waits_for_active_request; with this change the analogous H2 request fails on graceful shutdown instead of completing.

Useful? React with 👍 / 👎.

@quaesitor-scientiam quaesitor-scientiam Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct description of the semantic change — and intentional. The PR body and the comment in serve() both document this explicitly: "The only semantic change is that an h2 request in flight when the server stops is now interrupted rather than allowed to finish — which is acceptable at shutdown and is not relied on by any caller (the graceful 'wait for the active request' guarantee is HTTP/1.1-only)."

The per-frame mark/unmark approach that preserved the finish-in-flight behavior did so at the cost of a shared-mutex lock + O(n) list scan on every frame on every connection — contending one process-wide mutex on the hot reader path for no real benefit, since close_idle still interrupts the blocking frame read by shutting the fd regardless. Registering once per lifetime buys back that overhead.

The H2 server is also serial (SETTINGS_MAX_CONCURRENT_STREAMS = 1), so an "in-flight request" here means at most one call to handler.handle() that may not finish cleanly on shutdown — not a correctness issue for callers (no one awaits H2 request completion across Server.close()). test_server_tls_close_waits_for_active_request tests the H1 guarantee and is unaffected.

// The server is already shutting down; do not start serving.
return
}
defer {
if tracked {
c.idle_conns.unmark_idle(c.idle_handle)
}
}
c.read_client_preface()!
c.send_initial_settings()!
for !c.closing {
frame := c.read_idle_frame() or {
frame := c.read_frame() or {
// Treat a clean transport close as end of session.
return
}
Expand All @@ -88,33 +106,6 @@ fn (mut c H2ServerConn) should_track_idle_read() bool {
return c.idle_handle > 0 && c.idle_conns != unsafe { nil }
}

fn (mut c H2ServerConn) read_client_preface_idle() ! {
if !c.should_track_idle_read() {
c.read_client_preface()!
return
}
if !c.idle_conns.mark_idle(c.idle_handle) {
return error('h2 server: connection is shutting down')
}
defer {
c.idle_conns.unmark_idle(c.idle_handle)
}
c.read_client_preface()!
}

fn (mut c H2ServerConn) read_idle_frame() !H2Frame {
if !c.should_track_idle_read() {
return c.read_frame()!
}
if !c.idle_conns.mark_idle(c.idle_handle) {
return error('h2 server: connection is shutting down')
}
defer {
c.idle_conns.unmark_idle(c.idle_handle)
}
return c.read_frame()!
}

fn (mut c H2ServerConn) read_client_preface() ! {
c.fill_at_least(h2_client_preface.len)!
got := c.rbuf[..h2_client_preface.len].bytestr()
Expand Down
Loading