Skip to content

Commit 6e2d911

Browse files
net.http: run TLS server handshakes on worker threads (#27433 item 3) (#27552)
The TLS handshake ran inline on the single accept thread, so handshakes were serialized and a client that completed the TCP connect but stalled mid-handshake wedged the accept loop and delayed stop() by up to the handshake budget. This was previously deferred because concurrent handshakes would race on the listener's shared mbedtls config/RNG/key, but #27437 enabled MBEDTLS_THREADING_C on all platforms, so that shared state is now mutex-protected and each worker already handshakes on its own per-connection ssl context. Split SSLListener.accept_with_timeouts into accept_raw_with_timeout (raw TCP + ssl setup, no handshake) and a new SSLConn.complete_handshake; the accept thread now only does the fast raw accept (polled at 100ms) and each worker performs its connection handshake. The handshaking fd is idle-tracked, so close_idle can interrupt a stalled handshake and stop() is observed within ~accept_poll_timeout regardless of handshakes in flight. The accept thread enqueues each raw conn through an interruptible select (poll s.state) rather than a bare `ch <- conn`: the send, and the post-loop shutdown (ch.close + close_idle + ws.wait), all run on the accept thread while stop() only flips s.state from another thread. A slow-handshake flood that fills every worker and the channel buffer would otherwise block the accept thread on the send so it never reaches close_idle, delaying shutdown until a worker handshake timed out. On shutdown the accept thread closes the not-yet-queued conn itself (it was never idle-tracked). Supporting changes: - SSLConn.shutdown() is now idempotent (clears opened before freeing), so a worker-defer vs close_idle double-shutdown is a no-op rather than a double-free. - do_handshake_loop holds the handshake retry loop with no self-shutdown; server_handshake keeps the synchronous self-shutdown wrapper for accept(). - TlsIdleConnTracker.mark_idle dedups handles so close_idle cannot close the same fd twice within its own loop. Tests: double-shutdown idempotency guard; a parallel concurrent-handshake round trip; and a slow-handshake-flood shutdown test (a one-worker, one-slot server flooded with stalled handshakes must still stop() promptly, not wait out the handshake timeout). Verified on Windows and Linux (gcc). Co-authored-by: WOZCODE <contact@withwoz.com>
1 parent bde90fa commit 6e2d911

5 files changed

Lines changed: 268 additions & 32 deletions

File tree

vlib/net/http/server_tls_idle.v

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,13 @@ fn (mut t TlsIdleConnTracker) mark_idle(handle int) bool {
2828
if t.closing {
2929
return false
3030
}
31-
t.handles << handle
31+
// Dedup: the same handle can be marked twice (e.g. a worker marks it for the
32+
// handshake window and again per keep-alive request, or the OS recycles the
33+
// fd value). A duplicate would make close_idle shut down / close the same fd
34+
// twice within its own loop, so only track each handle once.
35+
if t.handles.index(handle) < 0 {
36+
t.handles << handle
37+
}
3238
return true
3339
}
3440

vlib/net/http/server_tls_notd_use_openssl.v

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ const tls_accept_poll_timeout = 100 * time.millisecond
1212

1313
// tls_handshake_timeout is the default value for Server.tls_handshake_timeout,
1414
// used as the fallback handshake budget when Server.accept_timeout is zero or
15-
// net.infinite_timeout. The handshake runs on the accept thread, so without a
16-
// finite bound a client that completes the TCP connect and then stalls
17-
// mid-handshake would wedge the accept loop forever.
15+
// net.infinite_timeout. The handshake now runs on a worker thread (the accept
16+
// thread only does the raw TCP accept, polled at tls_accept_poll_timeout, so
17+
// stop() is observed promptly regardless of handshakes); without a finite bound a
18+
// client that completes the TCP connect and then stalls mid-handshake would tie
19+
// up a worker indefinitely.
1820
const tls_handshake_timeout = 30 * time.second
1921

2022
fn tls_accept_timeouts(accept_timeout time.Duration, handshake_fallback time.Duration) (time.Duration, time.Duration) {
@@ -77,11 +79,14 @@ fn (mut s Server) listen_and_serve_tls() {
7779
}
7880
s.addr = addr
7981

82+
accept_poll_timeout, handshake_timeout := tls_accept_timeouts(s.accept_timeout,
83+
s.tls_handshake_timeout)
8084
ch := chan &mbedtls.SSLConn{cap: s.pool_channel_slots}
8185
mut idle_conns := &TlsIdleConnTracker{}
8286
mut ws := []thread{cap: s.worker_num}
8387
for wid in 0 .. s.worker_num {
84-
ws << new_tls_handler_worker(wid, ch, s.handler, s.max_keep_alive_requests, idle_conns)
88+
ws << new_tls_handler_worker(wid, ch, s.handler, s.max_keep_alive_requests,
89+
handshake_timeout, idle_conns)
8590
}
8691

8792
if s.show_startup_message {
@@ -94,10 +99,8 @@ fn (mut s Server) listen_and_serve_tls() {
9499
if s.on_running != unsafe { nil } {
95100
s.on_running(mut s)
96101
}
97-
accept_poll_timeout, handshake_timeout := tls_accept_timeouts(s.accept_timeout,
98-
s.tls_handshake_timeout)
99102
for s.state == .running {
100-
mut conn := listener.accept_with_timeouts(accept_poll_timeout, handshake_timeout) or {
103+
mut conn := listener.accept_raw_with_timeout(accept_poll_timeout) or {
101104
if s.state != .running {
102105
break
103106
}
@@ -109,7 +112,32 @@ fn (mut s Server) listen_and_serve_tls() {
109112
}
110113
continue
111114
}
112-
ch <- conn
115+
// Hand the raw (not-yet-handshaked) conn to a worker. Don't use a bare
116+
// blocking `ch <- conn`: the accept loop, this send, and the post-loop
117+
// shutdown (ch.close + close_idle + ws.wait) all run on this one thread,
118+
// while stop()/close() only flip s.state from another thread. Under a slow-
119+
// handshake flood -- every worker blocked in complete_handshake and the
120+
// channel buffer full of untracked conns -- a blocking send would wedge the
121+
// accept thread so it never re-checks s.state nor reaches close_idle(),
122+
// delaying shutdown until a worker handshake times out. Poll s.state via a
123+
// select timeout so shutdown is still observed promptly.
124+
mut queued := false
125+
for s.state == .running && !queued {
126+
select {
127+
ch <- conn {
128+
queued = true
129+
}
130+
accept_poll_timeout {
131+
// channel full; loop re-checks s.state
132+
}
133+
}
134+
}
135+
if !queued {
136+
// Shutting down before this conn could be handed off. It was never
137+
// mark_idle'd, so close_idle() won't see it; close it here (exactly
138+
// once -- no worker ever received it) to avoid leaking the fd.
139+
conn.shutdown() or {}
140+
}
113141
}
114142
ch.close()
115143
idle_conns.close_idle()
@@ -121,18 +149,20 @@ struct TlsHandlerWorker {
121149
id int
122150
ch chan &mbedtls.SSLConn
123151
max_keep_alive_requests int
152+
handshake_timeout time.Duration
124153
mut:
125154
idle_conns &TlsIdleConnTracker = unsafe { nil }
126155
pub mut:
127156
handler Handler
128157
}
129158

130-
fn new_tls_handler_worker(wid int, ch chan &mbedtls.SSLConn, handler Handler, max_keep_alive_requests int, idle_conns &TlsIdleConnTracker) thread {
159+
fn new_tls_handler_worker(wid int, ch chan &mbedtls.SSLConn, handler Handler, max_keep_alive_requests int, handshake_timeout time.Duration, idle_conns &TlsIdleConnTracker) thread {
131160
mut w := &TlsHandlerWorker{
132161
id: wid
133162
ch: ch
134163
handler: handler
135164
max_keep_alive_requests: max_keep_alive_requests
165+
handshake_timeout: handshake_timeout
136166
idle_conns: idle_conns
137167
}
138168
return spawn w.process_requests()
@@ -171,6 +201,24 @@ fn (mut w TlsHandlerWorker) handle_conn(mut conn mbedtls.SSLConn) {
171201
}
172202
conn.shutdown() or {}
173203
}
204+
// Run the TLS handshake here on the worker thread (the accept thread only does
205+
// the raw TCP accept), so handshakes proceed in parallel up to worker_num and a
206+
// client that stalls mid-handshake can't wedge the accept loop or delay stop().
207+
// mark_idle first so close_idle can interrupt a stalled handshake by force-
208+
// closing the fd, and so a conn handed off while the server is shutting down is
209+
// closed (mark_idle returns false) rather than handshaked. On success, unmark
210+
// before the serve path does its own idle marking, keeping each handle tracked
211+
// once. On any early return the defer above performs the single shutdown.
212+
if !w.idle_conns.mark_idle(conn.handle) {
213+
return
214+
}
215+
conn.complete_handshake(w.handshake_timeout) or {
216+
$if debug {
217+
eprintln('TLS handshake failed: ${err}')
218+
}
219+
return
220+
}
221+
w.idle_conns.unmark_idle(conn.handle)
174222
// If the TLS handshake negotiated HTTP/2 via ALPN, switch to the HTTP/2
175223
// driver; otherwise fall through to the existing HTTP/1.1 path unchanged.
176224
if conn.negotiated_alpn() == 'h2' {

vlib/net/http/server_tls_test.v

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,86 @@ fn test_server_tls_close_during_silent_handshake() {
269269
assert srv.status() == .closed
270270
}
271271

272+
// test_server_tls_close_under_handshake_flood guards the case where every worker
273+
// is stuck in a slow TLS handshake AND the channel buffer is full of untracked
274+
// raw conns, so the accept thread parks on `ch <- conn`. If the accept thread
275+
// could not poll s.state during that send it would never reach close_idle(), and
276+
// close() would hang until a worker handshake timed out. Each client sends a
277+
// valid ClientHello then stalls, so the server's handshake blocks waiting for the
278+
// client's next flight (a bare connect would instead fail the handshake fast and
279+
// free the worker). With one worker and a one-slot buffer this fills: conn 0 ->
280+
// the worker, conn 1 -> the buffer, conn 2 -> the blocked send.
281+
fn test_server_tls_close_under_handshake_flood() {
282+
$if use_openssl ? {
283+
eprintln('skipping: TLS server not implemented for -d use_openssl yet')
284+
return
285+
}
286+
port := pick_port() or {
287+
assert false, 'pick_port: ${err}'
288+
return
289+
}
290+
// A real 165-byte TLS 1.2 ClientHello captured from V's own mbedtls client.
291+
// The exact bytes (random/session id) are irrelevant; the server only needs a
292+
// structurally valid first flight to start the handshake and then block
293+
// waiting for the client's ClientKeyExchange, which never arrives.
294+
client_hello := [u8(0x16), 0x03, 0x03, 0x00, 0xa0, 0x01, 0x00, 0x00, 0x9c, 0x03, 0x03, 0x6a,
295+
0x3d, 0x34, 0x4c, 0x8f, 0x46, 0x64, 0xd8, 0xe9, 0x2d, 0x46, 0x16, 0xd3, 0xf2, 0x87, 0xe4,
296+
0xee, 0xc5, 0x57, 0x6c, 0x8e, 0xd7, 0x36, 0x19, 0x3b, 0x35, 0x7a, 0x01, 0x03, 0xde, 0x6e,
297+
0x64, 0x00, 0x00, 0x24, 0xc0, 0x2c, 0xc0, 0x2b, 0xc0, 0x30, 0xc0, 0x2f, 0xc0, 0x24, 0xc0,
298+
0x23, 0xc0, 0x28, 0xc0, 0x27, 0xc0, 0x0a, 0xc0, 0x09, 0xc0, 0x14, 0xc0, 0x13, 0x00, 0x9d,
299+
0x00, 0x9c, 0x00, 0x3d, 0x00, 0x3c, 0x00, 0x35, 0x00, 0x2f, 0x01, 0x00, 0x00, 0x4f, 0x00,
300+
0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, 0x00, 0x0b, 0x00, 0x02,
301+
0x01, 0x00, 0x00, 0x0d, 0x00, 0x1a, 0x00, 0x18, 0x08, 0x04, 0x08, 0x05, 0x08, 0x06, 0x04,
302+
0x01, 0x05, 0x01, 0x02, 0x01, 0x04, 0x03, 0x05, 0x03, 0x02, 0x03, 0x02, 0x02, 0x06, 0x01,
303+
0x06, 0x03, 0x00, 0x23, 0x00, 0x00, 0x00, 0x10, 0x00, 0x0e, 0x00, 0x0c, 0x02, 0x68, 0x32,
304+
0x08, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31, 0x00, 0x17, 0x00, 0x00, 0xff, 0x01,
305+
0x00, 0x01, 0x00]
306+
mut srv := &http.Server{
307+
addr: '127.0.0.1:${port}'
308+
cert: server_tls_cert
309+
cert_key: server_tls_key
310+
in_memory_verification: true
311+
// A finite accept_timeout doubles as the handshake budget (see
312+
// tls_accept_timeouts), so use a large one: each stalled handshake must
313+
// stay stuck long enough to keep the worker busy and form the wedge,
314+
// rather than time out in milliseconds. The accept loop still polls at the
315+
// 100ms tls_accept_poll_timeout cap regardless.
316+
accept_timeout: 8 * time.second
317+
worker_num: 1
318+
pool_channel_slots: 1
319+
handler: EchoHandler{}
320+
show_startup_message: false
321+
}
322+
t := spawn srv.listen_and_serve()
323+
srv.wait_till_running() or {
324+
srv.close()
325+
t.wait()
326+
assert false, 'server failed to start: ${err}'
327+
return
328+
}
329+
mut clients := []&net.TcpConn{}
330+
for _ in 0 .. 4 {
331+
mut c := net.dial_tcp('127.0.0.1:${port}') or { continue }
332+
c.write(client_hello) or {}
333+
clients << c
334+
}
335+
defer {
336+
for mut c in clients {
337+
c.close() or {}
338+
}
339+
}
340+
// Let the accept thread occupy the worker and buffer and park on the send.
341+
time.sleep(400 * time.millisecond)
342+
sw := time.new_stopwatch()
343+
srv.close()
344+
t.wait()
345+
// close_idle() force-closes the stuck handshake fds, so close() returns well
346+
// before the 8s handshake timeout — but only if the accept thread escaped the
347+
// blocked send to reach it.
348+
assert sw.elapsed() < 2 * time.second
349+
assert srv.status() == .closed
350+
}
351+
272352
fn test_server_tls_close_interrupts_idle_keep_alive() {
273353
$if use_openssl ? {
274354
eprintln('skipping: TLS server not implemented for -d use_openssl yet')
@@ -479,6 +559,72 @@ fn test_server_tls_close_interrupts_incomplete_h2_request() {
479559
assert srv.status() == .closed
480560
}
481561

562+
fn test_server_tls_parallel_handshakes() {
563+
$if use_openssl ? {
564+
eprintln('skipping: TLS server not implemented for -d use_openssl yet')
565+
return
566+
}
567+
port := pick_port() or {
568+
assert false, 'pick_port: ${err}'
569+
return
570+
}
571+
mut srv := &http.Server{
572+
addr: '127.0.0.1:${port}'
573+
cert: server_tls_cert
574+
cert_key: server_tls_key
575+
in_memory_verification: true
576+
accept_timeout: time.second
577+
handler: EchoHandler{}
578+
show_startup_message: false
579+
}
580+
t := spawn srv.listen_and_serve()
581+
srv.wait_till_running() or {
582+
srv.close()
583+
t.wait()
584+
assert false, 'server failed to start: ${err}'
585+
return
586+
}
587+
defer {
588+
srv.close()
589+
t.wait()
590+
}
591+
time.sleep(50 * time.millisecond)
592+
593+
// Fire many clients at once. Handshakes now run on the worker pool, so they
594+
// proceed concurrently against the listener's shared mbedtls config/RNG (safe
595+
// because MBEDTLS_THREADING_C is enabled on all platforms). Assert every
596+
// round-trip succeeds with the correct body — a thread-safety regression in
597+
// the shared handshake state would surface as a failed/garbled response.
598+
n := 16
599+
results := chan string{cap: n}
600+
for i in 0 .. n {
601+
spawn fn [results, port, i] () {
602+
resp := http.fetch(
603+
url: 'https://127.0.0.1:${port}/p${i}'
604+
enable_http2: false
605+
validate: false
606+
) or {
607+
results <- 'error: ${err}'
608+
return
609+
}
610+
if resp.status_code != 200 {
611+
results <- 'bad status: ${resp.status_code}'
612+
return
613+
}
614+
results <- resp.body
615+
}()
616+
}
617+
// Results arrive in nondeterministic order, so match on the common prefix
618+
// rather than a specific path index.
619+
mut ok := 0
620+
for _ in 0 .. n {
621+
body := <-results
622+
assert body.starts_with('tls hello /p'), 'unexpected response: ${body}'
623+
ok++
624+
}
625+
assert ok == n, 'expected ${n} successful concurrent handshakes, got ${ok}'
626+
}
627+
482628
fn test_server_tls_h2_negotiation() {
483629
$if use_openssl ? {
484630
eprintln('skipping: TLS server not implemented for -d use_openssl yet')

vlib/net/mbedtls/mbedtls_sslconn_shutdown_does_not_panic_test.v

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ fn server() ! {
2020
}
2121
eprintln('[+] Accepted connection')
2222
cli.shutdown()!
23+
// A second shutdown must be a harmless no-op (idempotent): SSLConn.shutdown
24+
// marks the conn closed before freeing, so this returns the "not open" error
25+
// here rather than double-freeing the mbedtls contexts (which would abort the
26+
// process). This guards the worker-defer-vs-close_idle double-shutdown race.
27+
mut second_shutdown_errored := false
28+
cli.shutdown() or { second_shutdown_errored = true }
29+
assert second_shutdown_errored, 'second shutdown should be a no-op returning an error, not a double-free'
30+
eprintln('[+] Second shutdown was a clean no-op')
2331
}
2432

2533
@[if network ?]

0 commit comments

Comments
 (0)