fix: close the tls server config a listen loop built

Makefile

@@ -1149,7 +1149,8 @@ MEMCHECK_CASES := \
 	tests/cases/test_yaml_derived_decode \
 	tests/cases/test_concurrent_group \
 	tests/cases/test_map_value_ownership \
-	tests/cases/test_web_session_ownership
+	tests/cases/test_web_session_ownership \
+	tests/cases/test_tls_server_config_release
 
 memcheck: build
 	@echo "--- memcheck (valgrind, curated) ---"

docs/tls.md

@@ -46,11 +46,44 @@ closing after the connection is up is deliberate, not a trick. a client config
 is read only while the handshake runs — the roots to verify the peer chain, the
 alpn list to offer, the client certificate to present. once the handshake
 returns, the connection holds its own keys and never looks the config up again,
-so the config has no job left. the same goes for `tls.listen`: the listener owns
-its server config from that point on and `Listener.close()` releases it.
+so the config has no job left.
+
+a server config is the same rule with the timing spelled out. `tls.listen`
+records the config against the listening socket, but that is a borrow, not a
+transfer: `Listener.close()` gives the borrow back, and whoever called
+`server_config()` is still the one who closes the config.
+
+the timing is what makes a server different in practice. an accept loop hands
+each socket to its own task, and that task reads the certificate and the private
+key out of the registry when its own handshake reaches them — which can be a
+whole handshake timeout after the accept that spawned it returned. so a server
+closes its config once it has drained, never before:
+
+```pith
+cfg := tls.server_config("certs/server.crt", "certs/server.key")!
+listener := tls.listen(host, 443, cfg)!
+# ... accept, spawning a task per connection, until a shutdown is requested ...
+listener.close()
+drained := shutdown.drain_default()
+cfg.close()
+```
+
+closing before the drain does not corrupt anything — a handle is never reissued,
+so the handshake that loses the race fails cleanly rather than reaching for
+another config's key — but it does turn working connections into failed ones on
+the way out, which is a worse shutdown than the leak it was fixing.
+
+the std servers do this for you: `App.listen_tls`, `http2.listen_h2_tls` and its
+streaming twin each build a config, serve on it, and close it after their drain,
+so a program that calls one of those has nothing to close.
+`tls.open_server_configs()` counts the server configs still holding a
+certificate, and on a healthy process the number is flat.
 
 `close()` is idempotent and safe on a config that holds nothing, so a failure
-path can close unconditionally on its way out.
+path can close unconditionally on its way out — as long as it is a path where
+nothing is still handshaking on it. that caveat is the whole reason a server
+closes after its drain rather than in a `defer`, which would fire on the paths
+that never drain too.
 
 the functions that build a config for you close it for you — `tls.dial`,
 `http.get` and friends, `http2.connect`. the `_with_config` variants never

std/net/http2/server.pith

@@ -1372,7 +1372,13 @@ fn serve_h2_tls_slot(listener_handle: Int, fd: Int, handler: fn(http.HttpRequest
 # racing connections — from taking the accept loop down with it.
 pub fn listen_h2_tls(host: String, port: Int, cert: String, key: String, handler: fn(http.HttpRequestBytes) -> http.HttpResponse) -> Int!:
     config := tls.server_config(cert, key)!.with_alpn(["h2", "http/1.1"])
-    listener := tls.listen(host, port, config)!
+    bound := tls.listen(host, port, config)
+    if bound.is_err:
+        # the bind never happened, so nothing borrowed the config and it is
+        # still this call's to close.
+        config.close()
+        fail bound.err
+    listener := bound.ok
     shutdown.register_listener(listener.handle)
     defer release_tls_listener(listener)
     mut failures := 0
@@ -1382,14 +1388,23 @@ pub fn listen_h2_tls(host: String, port: Int, cert: String, key: String, handler
             if shutdown.requested():
                 break
             failures = failures + 1
+            # a give-up here leaves the config open on purpose: nothing drains
+            # on this path, and a task already mid-handshake still reads the
+            # certificate and key out of the registry.
             tcp.back_off_after_accept_failure(failures, accepted.err)!
             continue
         failures = 0
         connection_slots.acquire()
         spawn serve_h2_tls_slot(listener.handle, accepted.ok, handler)
     # free the port before draining (see listen_h2c above).
     release_tls_listener(listener)
-    return shutdown.drain_default()
+    drained := shutdown.drain_default()
+    # this call built the config, so this call closes it — and only here, once
+    # the drain has finished. the listener merely borrowed it, and each
+    # connection task reads the certificate and key out of the registry when its
+    # own handshake gets that far. see docs/tls.md.
+    config.close()
+    return drained
 
 # --- streaming entry points ---
 #
@@ -1472,7 +1487,13 @@ fn serve_h2_tls_streaming_slot(listener_handle: Int, fd: Int):
 pub fn listen_h2_tls_streaming(host: String, port: Int, cert: String, key: String, handler: fn(ServerStream) -> Int) -> Int!:
     stream_handler_fn = handler
     config := tls.server_config(cert, key)!.with_alpn(["h2", "http/1.1"])
-    listener := tls.listen(host, port, config)!
+    bound := tls.listen(host, port, config)
+    if bound.is_err:
+        # the bind never happened, so nothing borrowed the config and it is
+        # still this call's to close.
+        config.close()
+        fail bound.err
+    listener := bound.ok
     shutdown.register_listener(listener.handle)
     defer release_tls_listener(listener)
     mut failures := 0
@@ -1482,14 +1503,20 @@ pub fn listen_h2_tls_streaming(host: String, port: Int, cert: String, key: Strin
             if shutdown.requested():
                 break
             failures = failures + 1
+            # a give-up here leaves the config open on purpose: nothing drains
+            # on this path, and a task already mid-handshake still reads the
+            # certificate and key out of the registry.
             tcp.back_off_after_accept_failure(failures, accepted.err)!
             continue
         failures = 0
         connection_slots.acquire()
         spawn serve_h2_tls_streaming_slot(listener.handle, accepted.ok)
     # free the port before draining (see listen_h2c above).
     release_tls_listener(listener)
-    return shutdown.drain_default()
+    drained := shutdown.drain_default()
+    # this call built the config, so this call closes it — see listen_h2_tls.
+    config.close()
+    return drained
 
 # --- offline frame tests ---
 #

std/net/tls.pith

@@ -78,6 +78,12 @@ fn config_drop[V](values: Map[Int, V], handle: Int):
         values.remove(handle)
     native_config_mu.unlock()
 
+fn config_len[V](values: Map[Int, V]) -> Int:
+    native_config_mu.lock()
+    n := values.len()
+    native_config_mu.unlock()
+    return n
+
 # the root bundle caches, guarded by native_config_mu like the registry above.
 #
 # a client config's trust anchors are the most expensive thing about it. the
@@ -1370,6 +1376,16 @@ pub fn listener_from_handle(handle: Int) -> Listener:
 pub fn release_listener_config(listener: Listener):
     config_drop(native_listener_config, listener.handle)
 
+# how many server configs are open right now: one per server_config() that has
+# not been closed yet.
+#
+# a server builds one config per listener, so on a running process this is flat
+# and small. a number that climbs is a config leak, and the leak is not free —
+# an open server config pins its certificate pem and its private key der in the
+# registry for as long as it stays open.
+pub fn open_server_configs() -> Int:
+    return config_len(native_server_cert_pem)
+
 # how many server handshakes have failed since this process started.
 #
 # a handshake fails when a peer opens a connection and then hangs up, sends
@@ -1447,8 +1463,16 @@ impl Config:
     # closing is safe as soon as the handshakes made from the config have
     # returned. a client config is read only while a handshake runs, and the
     # connection it produces keeps its own keys rather than a reference back
-    # here. a listener is the exception: it holds a server config for as long as
-    # it is listening, and Listener.close() is what releases that one.
+    # here.
+    #
+    # a server config is the same rule read carefully. `listen` records the
+    # handle against the listening socket, but that is a borrow, not a transfer:
+    # Listener.close() gives back the borrow, and whoever called server_config()
+    # still closes the config. the borrow is what makes the timing matter. an
+    # accept loop hands each socket to its own task, and that task reads the
+    # certificate and the key out of this registry when its handshake reaches
+    # them — which can be a whole handshake timeout after the accept returned.
+    # so a server closes its config after the drain, never before it.
     #
     # every map a config can write to is dropped here. missing one would move
     # the leak rather than fix it, so there is a test that walks all of them.
@@ -1698,6 +1722,31 @@ test "closing a client config empties every registry map it wrote to":
     # closing again is a no-op, so a failure path can close unconditionally.
     cfg.close()
 
+test "closing a server config empties every registry map it wrote to":
+    before := open_server_configs()
+    cfg := server_config("tests/live/fixtures/localhost.crt", "tests/live/fixtures/localhost.key")!.with_alpn(["pith.rpc"]).request_client_ca_file("tests/live/fixtures/localhost-ca.crt")!
+    assert(is_native_server_config(cfg))
+    assert_eq(open_server_configs(), before + 1)
+
+    cfg.close()
+
+    # the server half of the client test above. the certificate pem and the
+    # private key der are the expensive entries here, so a close that misses one
+    # keeps a key in memory for the rest of the process.
+    assert(not config_has(native_server_cert_pem, cfg.handle))
+    assert(not config_has(native_server_key_der, cfg.handle))
+    assert(not config_has(native_server_client_ca_pem, cfg.handle))
+    assert(not config_has(native_server_client_ca_optional, cfg.handle))
+    assert(not config_has(native_config_alpn, cfg.handle))
+    assert(not is_native_server_config(cfg))
+    assert_eq(open_server_configs(), before)
+
+    # a closed server config fails a handshake rather than serving one with
+    # whatever is left in the registry — handles are never reissued.
+    assert(native_server_certificate_chain(cfg).is_err)
+
+    cfg.close()
+
 test "a root bundle is parsed once and shared by every config that trusts it":
     first := client_config_with_ca_file("tests/live/fixtures/localhost-ca.crt")!
     second := client_config_with_ca_file("tests/live/fixtures/localhost-ca.crt")!

std/web.pith

@@ -570,7 +570,14 @@ impl App:
     fn listen_tls(host: String, port: Int, cert: String, key: String) -> Int!:
         app := self
         config := tls.server_config(cert, key)!.with_alpn(["h2", "http/1.1"])
-        listener := tls.listen(host, port, config)!
+        bound := tls.listen(host, port, config)
+        if bound.is_err:
+            # the bind never happened, so nothing borrowed the config and it is
+            # still this call's to close. a server that retries a taken port
+            # would otherwise leak a certificate and a key per attempt.
+            config.close()
+            fail bound.err
+        listener := bound.ok
         shutdown.register_listener(listener.handle)
         defer release_tls_listener(listener)
         mut failures := 0
@@ -580,13 +587,23 @@ impl App:
                 if shutdown.requested():
                     break
                 failures = failures + 1
+                # a give-up here leaves the config open on purpose: nothing
+                # drains on this path, and a task already mid-handshake still
+                # reads the certificate and key out of the registry.
                 tcp.back_off_after_accept_failure(failures, accepted.err)!
                 continue
             failures = 0
             spawn handle_tls_socket(app, listener.handle, accepted.ok)
         # free the port before draining (see the plaintext listener above).
         release_tls_listener(listener)
-        return shutdown.drain_default()
+        drained := shutdown.drain_default()
+        # this call built the config, so this call closes it — and only here,
+        # once the drain has finished. the listener merely borrowed it, and each
+        # connection task reads the certificate and key out of the registry when
+        # its own handshake gets that far, which is long after the accept that
+        # spawned it returned. see docs/tls.md.
+        config.close()
+        return drained
 
 # --- tests ---
 

tests/cases/test_tls_server_config_release.pith

@@ -0,0 +1,129 @@
+# a tls server closes the config it built, and not a moment too soon.
+#
+# the two halves of the rule, in one run against a real listener:
+#
+#   1. the config survives the drain. this test gets a connection accepted and
+#      counted as in flight, *then* asks for a shutdown, and only then sends the
+#      client hello. the server's connection task reads the certificate and the
+#      private key out of the tls registry at that point — well after the accept
+#      loop stopped — so a listen loop that closed its config before draining
+#      would fail this handshake.
+#   2. the config is closed once the drain is over. tls.open_server_configs()
+#      counts the configs still holding a certificate; it has to come back to
+#      zero after the listen call returns, or the certificate pem and the key
+#      der sit in the registry for the rest of the process.
+#
+# nothing here waits on a fixed sleep. the connect retries until the listener is
+# bound, and shutdown.inflight() is what says the server has taken the
+# connection, so the ordering is observed rather than hoped for.
+import std.web as web
+import std.net.http as http
+import std.net.tls as tls
+import std.net.tcp as tcp
+import std.shutdown as shutdown
+import std.time as time
+from std.io import TcpStream
+
+PORT := 18473
+CERT := "tests/live/fixtures/localhost.crt"
+KEY := "tests/live/fixtures/localhost.key"
+CA := "tests/live/fixtures/localhost-ca.crt"
+
+# how many 10ms turns a poll gives up after. generous, because it only ever runs
+# to completion on a broken build; a healthy one leaves after a turn or two.
+POLL_TURNS := 500
+
+mut server_returned := false
+
+fn hello(req: web.Request) -> http.HttpResponse:
+    return http.text(200, "hello")
+
+# the server under test: a one-route app on a real tls listener. the flag is how
+# main knows the listen call returned, so it can check the registry afterwards
+# without guessing at a delay.
+fn serve_bg() -> Int:
+    app := web.new().get("/", hello)
+    code := app.listen_tls("127.0.0.1", PORT, CERT, KEY) catch 0
+    server_returned = true
+    return code
+
+# connect to the listener, retrying until it is bound. returns -1 if it never
+# comes up.
+fn connect_when_bound() -> Int:
+    mut turns := 0
+    while turns < POLL_TURNS:
+        attempt := tcp.connect("127.0.0.1", PORT)
+        if attempt.is_ok:
+            return attempt.ok
+        turns = turns + 1
+        time.delay(10)
+    return 0 - 1
+
+# wait until the server has accepted the connection and counted it as in flight.
+fn wait_for_inflight() -> Bool:
+    mut turns := 0
+    while turns < POLL_TURNS:
+        if shutdown.inflight() > 0:
+            return true
+        turns = turns + 1
+        time.delay(10)
+    return false
+
+fn wait_for_server_return() -> Bool:
+    mut turns := 0
+    while turns < POLL_TURNS:
+        if server_returned:
+            return true
+        turns = turns + 1
+        time.delay(10)
+    return false
+
+# the response status line, or "" when the exchange failed.
+fn request_over(conn: tls.Conn) -> String:
+    crlf := chr(13) + chr(10)
+    sent := conn.write_all("GET / HTTP/1.1" + crlf + "Host: localhost" + crlf + "Connection: close" + crlf + crlf)
+    if sent.is_err:
+        return ""
+    reply := conn.read(256)
+    if reply.is_err:
+        return ""
+    lines := reply.ok.split(crlf)
+    if lines.len() == 0:
+        return ""
+    return lines[0]
+
+fn main() -> Int!:
+    shutdown.reset()
+    shutdown.set_drain_deadline(5000)
+    print("open server configs at start: " + tls.open_server_configs().to_string())
+
+    spawn serve_bg()
+
+    # a tcp connection the server accepts and counts, with no client hello on it
+    # yet. from here the server task is parked reading a handshake record.
+    fd := connect_when_bound()
+    print("connected: " + (fd > 0).to_string())
+    print("server took the connection: " + wait_for_inflight().to_string())
+
+    # now stop the server. the accept loop breaks, frees the port, and drains —
+    # with this connection's handshake still to happen.
+    shutdown.request()
+
+    # the listener offers alpn, so a client that offers none is refused. ask for
+    # http/1.1 and stay off the http/2 path this test does not need.
+    client_cfg := tls.client_config_with_ca_file(CA)!.with_alpn(["http/1.1"])
+    defer client_cfg.close()
+    handshaken := tls.client(TcpStream(fd), "localhost", client_cfg)
+    print("handshake after the shutdown request: " + handshaken.is_ok.to_string())
+    if handshaken.is_err:
+        print("handshake error: " + handshaken.err)
+        return 1
+
+    # the server config is still open here, because the drain is still running.
+    print("open server configs during the drain: " + tls.open_server_configs().to_string())
+    print("served during the drain: " + request_over(handshaken.ok))
+    handshaken.ok.close()
+
+    print("listen returned: " + wait_for_server_return().to_string())
+    print("open server configs after: " + tls.open_server_configs().to_string())
+    return 0

tests/expected/test_tls_server_config_release.txt

@@ -0,0 +1,8 @@
+open server configs at start: 0
+connected: true
+server took the connection: true
+handshake after the shutdown request: true
+open server configs during the drain: 1
+served during the drain: HTTP/1.1 200 OK
+listen returned: true
+open server configs after: 0