From 143e89700bb63ea5073f06793bf2e95d36d81492 Mon Sep 17 00:00:00 2001 From: Richard Wheeler Date: Mon, 15 Jun 2026 19:43:51 -0400 Subject: [PATCH] net.http: track h2 server connections for idle shutdown once, not per frame (#27435) The HTTP/2 server registered each connection's fd in the TLS idle-conn tracker around every frame read (mark_idle before, unmark_idle after), so the shared tracker mutex -- and an O(n) handle-list scan in unmark -- was hit on the hot path for every frame on every connection. Register the connection once for its serve lifetime instead. The reader spends almost all of its time blocked in a frame read, so the per-frame churn bought nothing: close_idle still interrupts the reader on shutdown by shutting the fd down. 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 and unaffected. Refs #27433. Co-authored-by: Claude Opus 4.8 --- vlib/net/http/h2_server.v | 49 ++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/vlib/net/http/h2_server.v b/vlib/net/http/h2_server.v index 461e674ea..fa8f2a11a 100644 --- a/vlib/net/http/h2_server.v +++ b/vlib/net/http/h2_server.v @@ -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) { + // 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 } @@ -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() -- 2.39.5