net.http: run TLS server handshakes on worker threads (#27433 item 3) - #27552
Merged
JalonSolov merged 1 commit intoJun 25, 2026
Merged
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
quaesitor-scientiam
force-pushed
the
tls-server-parallel-handshake
branch
from
June 24, 2026 19:02
9d662f9 to
bf730df
Compare
Contributor
Author
|
Rebased onto current master to clear unrelated CI failures. The red jobs were 13 |
Contributor
Author
|
Analysis of CI failures are not caused by this PR |
Collaborator
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf730df187
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…em 3) 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 vlang#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>
quaesitor-scientiam
force-pushed
the
tls-server-parallel-handshake
branch
from
June 25, 2026 14:26
bf730df to
e145b0e
Compare
Collaborator
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
JalonSolov
pushed a commit
that referenced
this pull request
Jun 28, 2026
…down test guards (#27559) * net.http,net.mbedtls: harden TLS parallel-handshake + idempotent-shutdown test guards Follow-up to #27552 (run TLS server handshakes on worker threads). Test-only; no production change. server_tls_test.v: test_server_tls_parallel_handshakes used a non-blocking EchoHandler, so it passed even if handshakes ran fully serially (the accept loop accepts one conn at a time and worker_num can be 1 on low-core CI) - it only proved the shared mbedtls config/RNG survived serial handoff. Now force worker_num:4 and use a width-2 barrier: BlockingHandler signals on entry then parks on release, and the test refuses to release until >=2 handlers are parked at once. That can only happen if >=2 workers each completed a handshake and ran concurrently; with a single worker the 2nd arrival never comes and the overlap wait times out, failing the test (verified: worker_num:1 fails the overlap assert, worker_num:4 passes). Parking frees the worker, so overlap is observable even on a single core. mbedtls_sslconn_shutdown_does_not_panic_test.v: the double-shutdown check only asserted the 2nd shutdown returned *an* error; a reintroduced double-free could still error on a forgiving allocator. Assert the specific "connection was not open" sentinel so the test proves the idempotent !opened early-return path was taken, not a second pass through the mbedtls frees. Co-Authored-By: WOZCODE <contact@withwoz.com> * net.http: gate parallel-handshake test on handshake-layer concurrency The parallel-handshakes test counted overlapping request handlers, which only proves two handlers ran at once — a serial-accept-then-queue design produces that too (the accept thread completes handshakes one after another and hands both finished connections to workers that park together). It therefore could not catch the regression it was meant to guard (Codex review on #27559, discussion_r3478571393). Move the discriminator to the handshake layer: wedge `stall` workers with raw TCP connections that connect but never send a ClientHello (each blocks inside complete_handshake), then fire `live` real HTTPS clients. With per-worker parallel handshakes the free workers service the live clients in well under a second; a serial-accept regression wedges the accept thread on the first stalled connection and cannot service the live clients until a stalled handshake hits its 10s budget. The check is time-bounded (live phase < 6s) so the regression fails the test instead of merely running ~20s slower. Verified: good path (worker_num 4) passes repeatedly at sub-second live phase; forcing the pool below stall+live reproduces the regression (live phase 19.6s, fails the 6s bound). Co-Authored-By: WOZCODE <contact@withwoz.com> * net.http: force fresh TLS connection per live client in handshake test The two live clients in the parallel-handshake test used the shared default transport's keep-alive pool. If one client finished and returned its connection to the pool before the other checked one out, the second would reuse it and the test could pass after only a single live TLS handshake — no longer proving two concurrent completing handshakes against the shared mbedtls config/RNG (Codex review on #27559, discussion_r3484264670). Set disable_connection_reuse on the live fetches so each opens its own TLS connection (and sends Connection: close), guaranteeing two genuine concurrent handshakes. Still green; live phase stays sub-second, well under the 6s bound. Co-Authored-By: WOZCODE <contact@withwoz.com> * net.http: bound parallel-handshake wait with a select deadline The previous version measured elapsed time and asserted it was < 6s only after draining the results channel. On the serialized-handshake regression path that drain blocked on the client's own TLS handshake: http.fetch (Request.ssl_do) dials and completes the handshake before req.read_timeout applies and retries on socket errors, so the live clients sat in the SSL backend handshake timeout (and retries) for ~19s before the assert ran. The test still failed, but slowly (Codex review on #27559, discussion_r3484599003). Bound the wait itself: collect the live results inside a single 6s budget via a select deadline, so a regression fails promptly at ~6s instead of after the client timeout. Also set max_retries: 1 on the live fetches so a regressed handshake surfaces as one timed-out request rather than retry-amplifying the teardown. Verified: good path passes (sub-second live phase); the regression sim now trips the assert at the 6s budget ("got 0") instead of ~19s, cutting the failing-run time roughly in half. Co-Authored-By: WOZCODE <contact@withwoz.com> --------- Co-authored-by: WOZCODE <contact@withwoz.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Runs TLS server handshakes on the worker pool instead of inline on the single
accept thread. This is the last open item (item 3) of #27433; items 1, 2, 4
and 5 landed via #27434, #27542 and #27435.
Why
The handshake ran inline in the accept loop, so:
handshake-heavy (many short TLS connection) workloads; and
wedged the accept loop and delayed
stop()by up to the handshake budget(~30s default).
This was deferred earlier because running handshakes concurrently would race on
the listener's shared mbedtls config /
ctr_drbgRNG / RSA key-blinding state.That blocker is gone: #27437 enabled
MBEDTLS_THREADING_Con all platforms(Linux/BSD/macOS pthread, Windows
THREADING_ALT), so the shared state is nowmutex-protected, and each worker already handshakes on its own per-connection
sslcontext.How
SSLListener.accept_with_timeoutsintoaccept_raw_with_timeout(rawTCP accept + non-blocking SSL setup, no handshake) and a new
SSLConn.complete_handshake(drives the handshake, then restores blockingmode/bio).
accept_with_timeoutsis kept as a thin wrapper, soaccept()andany external callers are unchanged.
worker performs its connection's handshake. The handshaking fd is registered in
the idle tracker first, so
close_idlecan interrupt a stalled handshake andstop()is observed within ~accept_poll_timeoutregardless of handshakes inflight.
Supporting correctness changes:
SSLConn.shutdown()is now idempotent (clearsopenedbefore freeing), soa worker-defer vs
close_idledouble-shutdown is a harmless no-op rather than adouble-free of the mbedtls contexts.
do_handshake_loopholds the handshake retry loop with no self-shutdown;server_handshakekeeps the synchronous self-shutdown wrapper foraccept().TlsIdleConnTracker.mark_idlededups handles soclose_idlecannot shut down /close the same fd twice within its own loop.
The single-close contract holds on every worker path (mark_idle-false,
handshake-error, Windows force-close mid-handshake, success → serve), and the
Windows
was_force_closed→owns_socket = falseownership transfer from #27542is preserved.
Tests
worker-side interrupt path.
Verified on Windows, Linux (native, gcc) and macOS: full
vlib/net/mbedtls/andvlib/net/http/suites pass, withserver_tls_teststable across repeated runs.
🧙 Built with WOZCODE