Skip to content

Commit a0f8da7

Browse files
net.http: free TLS resources for force-closed idle connections
The previous fix skipped conn.shutdown() entirely for an idle TLS connection that close_idle force-closed on Windows, to avoid closing the socket a second time. But SSLConn.shutdown() does two jobs: it frees the mbedtls TLS resources (SSL context, config, certs, RNG state, ALPN allocation) AND closes the socket. Skipping it leaked all of those for every force-closed idle connection at server shutdown. Split the two concerns instead of bypassing cleanup: when close_idle already owns the socket close (was_force_closed), relinquish socket ownership (conn.owns_socket = false) and still call conn.shutdown(). shutdown() then frees the TLS resources but skips net.close, so the fd is still closed exactly once (by close_idle) and nothing leaks. On non-Windows was_force_closed is always false and the worker remains the sole closer, unchanged. Found by Codex on #27542. Co-Authored-By: WOZCODE <contact@withwoz.com>
1 parent 31405a6 commit a0f8da7

2 files changed

Lines changed: 23 additions & 16 deletions

File tree

vlib/net/http/server_tls_idle.v

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ mut:
1313
handles []int
1414
closing bool
1515
// closed records handles that close_idle force-closed on Windows. The woken
16-
// worker consults was_force_closed and skips its own conn.shutdown so the fd
17-
// is closed exactly once. Empty on non-Windows (close_idle only shuts the fd
18-
// down there, leaving the worker the sole closer).
16+
// worker consults was_force_closed and relinquishes socket ownership (so its
17+
// conn.shutdown frees the TLS resources without closing the fd again), keeping
18+
// the fd closed exactly once. Empty on non-Windows (close_idle only shuts the
19+
// fd down there, leaving the worker the sole closer).
1920
closed []int
2021
}
2122

@@ -44,11 +45,12 @@ fn (mut t TlsIdleConnTracker) unmark_idle(handle int) {
4445

4546
// was_force_closed reports whether close_idle has already closed `handle`
4647
// (Windows force-close to wake a worker blocked in select). The woken worker
47-
// must then skip its own conn.shutdown: a second close would race process-wide
48-
// SOCKET reuse — the value can be reused by ANY socket the process opens, not
49-
// just this server's accept loop — and could close an unrelated socket. Always
50-
// false on non-Windows, where close_idle only shuts the fd down and the worker
51-
// remains the sole closer.
48+
// must then relinquish socket ownership before its own conn.shutdown, which
49+
// then frees the TLS resources without closing the fd again: a second close
50+
// would race process-wide SOCKET reuse — the value can be reused by ANY socket
51+
// the process opens, not just this server's accept loop — and could close an
52+
// unrelated socket. Always false on non-Windows, where close_idle only shuts
53+
// the fd down and the worker remains the sole closer.
5254
fn (mut t TlsIdleConnTracker) was_force_closed(handle int) bool {
5355
$if windows {
5456
t.mu.lock()
@@ -74,8 +76,9 @@ fn (mut t TlsIdleConnTracker) close_idle() {
7476
// select() — only closesocket() (net.close, below) does — so we must
7577
// force-close idle handles to wake their workers. Record ownership of
7678
// those closes here, under the lock, so a worker that wakes (or exits for
77-
// another reason) sees it via was_force_closed and skips its own
78-
// conn.shutdown. Without that the fd would be closed twice, and between
79+
// another reason) sees it via was_force_closed and relinquishes socket
80+
// ownership instead of closing the fd again. Without that the fd would be
81+
// closed twice, and between
7982
// the two closes the SOCKET value can be reused by any socket the process
8083
// opens, so the second close could hit an unrelated socket.
8184
t.closed << handles

vlib/net/http/server_tls_notd_use_openssl.v

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -159,13 +159,17 @@ fn (mut w TlsHandlerWorker) handle_conn(mut conn mbedtls.SSLConn) {
159159
w.idle_conns.unmark_idle(conn.handle)
160160
}
161161
// close_idle may have already closed conn.handle on Windows (a forced
162-
// closesocket to wake this worker from a blocked read). Closing it again
163-
// would race process-wide SOCKET reuse and could close an unrelated
164-
// socket, so skip our close when close_idle owns it. On non-Windows this
165-
// is always false and the worker remains the sole closer.
166-
if !w.idle_conns.was_force_closed(conn.handle) {
167-
conn.shutdown() or {}
162+
// closesocket to wake this worker from a blocked read). conn.shutdown()
163+
// both frees the mbedtls TLS resources (SSL ctx, config, certs, RNG,
164+
// ALPN alloc) AND closes the socket; we must always do the former but
165+
// must not close the socket a second time — that would race process-wide
166+
// SOCKET reuse and could close an unrelated socket. So relinquish socket
167+
// ownership and still call shutdown for the TLS cleanup. On non-Windows
168+
// was_force_closed is always false and the worker remains the sole closer.
169+
if w.idle_conns.was_force_closed(conn.handle) {
170+
conn.owns_socket = false
168171
}
172+
conn.shutdown() or {}
169173
}
170174
// If the TLS handshake negotiated HTTP/2 via ALPN, switch to the HTTP/2
171175
// driver; otherwise fall through to the existing HTTP/1.1 path unchanged.

0 commit comments

Comments
 (0)