fix(tls): reuse SSL_CTX across tls.connect({ socket }) upgrades

Closed
TU
6mo ago
+367-21

Disclosure / disclaimer. This PR was produced by Claude Code. The author (@tuan3w) does not know Zig and is not familiar with BoringSSL or the Bun internals touched here — only reviewed the model's output at a high level. Please treat the code as model-generated and review openssl.c + the refcount and cache logic carefully. If the approach is sound but the code needs rewriting by someone who actually knows the codebase, feel free to close and redo in a proper PR. Benchmark numbers and the reproducer are real.

Summary

Fixes #12117 (the MongoDB / mongoose / mysql2 / any-starttls memory-leak reports). The user-visible bug is RSS climbing indefinitely under any workload that repeatedly calls tls.connect({ socket }); on Linux + Docker this ends in OOM kills every few days.

Root cause: every socket.upgradeTLS() call unconditionally routes through us_create_bun_ssl_socket_context → SSL_CTX_new + cert/cipher/DH parsing (~50–100KB per call). Even after the two earlier fixes in this area — #26766 (wrapper has_pending_activity) and #29451 (JSRef wrapper lifecycle) — each upgrade still mints a fresh SSL_CTX underneath. The Zig-level bookkeeping is balanced and leaks --atExit reports only 992 bytes genuinely unreachable after 1000 cycles, yet RSS still climbs because the per-call allocation fan-out fragments the allocator arena and, on glibc ptmalloc, that memory never returns to the OS.

This PR caches the SSL_CTX by SSLConfig.contentHash() so two upgrades with the same TLS config share one SSL_CTX across the process lifetime. This matches how Chromium's network stack runs every outbound TLS connection off a single process-wide SSL_CTX singleton (ssl_client_socket_impl.cc) and how Rust's rust-openssl documents it: "Applications commonly configure a single SslContext that is shared by all of its SslStreams." (See the verified Prior art section below — the original version of this PR cited Node.js as precedent, which turned out to be wrong; Node actually creates a new SecureContext per connection.)

Benchmark

The core measurement: the same debug binary on the same machine, doing 1000 tls.connect({ socket }) upgrade cycles. The only difference is whether lookupCachedSSLCtx returns a hit; everything else (GC settings, certs, server loop, reproducer, drain logic) is identical. Two fresh runs of each:

ConfigRSS after 1000 cyclesWall timeSSL_CTX_new calls
Cache on (this PR)167.8 MB / 167.9 MB17.5 s / 17.7 s1
Cache off (same HEAD, one branch disabled)1052.3 MB / 1051.6 MB30.2 s / 30.2 s1000

6.3× lower RSS, 1.7× faster wall time. Variance between the two runs per side is under 1% — this is a real, reproducible effect, not allocator noise.

What about release builds on macOS?

For reference, system bun 1.3.13 release on the same reproducer reports 162 MB at 1000 cycles — close to the cached debug number. That's not an argument against the fix. macOS's libmalloc (nano + small zones) is aggressive about returning freed pages to the OS, so the allocation churn in the pre-PR release build plateaus even though the underlying SSL_CTX_new-per-cycle churn is still happening. The user-felt leak in #12117 is consistently reported from Linux + Docker, where glibc ptmalloc retains per-thread arenas and pages almost never go back to the OS. On glibc the 1000 SSL_CTX_new allocations per 1000 cycles compound directly into arena high-watermark growth.

The structural claim — "we go from N SSL_CTX_new per workload to 1" — holds identically on every allocator. The RSS impact is platform-dependent only because allocators differ in how quickly they return freed pages.

Zig-level lifecycle counters stay balanced either way (tls Δ=0, raw Δ=1, ctx_deinit = 2×ctx_create), so this is strictly an allocation-volume fix — there is no new resource to leak.

Reproducer script (drop next to your certs and run)
// repro-for-leaks.ts
//   CYCLES=1000 INTERVAL_MS=10 bun repro-for-leaks.ts
import * as net from "node:net";
import * as tls from "node:tls";
import * as fs from "node:fs";
import { join } from "node:path";
import { heapStats } from "bun:jsc";

const N = Number(process.env.CYCLES ?? 200);
const INTERVAL_MS = Number(process.env.INTERVAL_MS ?? 20);
const key = fs.readFileSync(join(import.meta.dir, "certs/mongo-key.pem"));
const cert = fs.readFileSync(join(import.meta.dir, "certs/mongo-cert.pem"));

const server = tls.createServer({ key, cert }, s => {
  s.on("data", d => { try { s.write(d) } catch {} });
  s.on("error", () => {});
});
await new Promise<void>(r => server.listen(0, "127.0.0.1", () => r()));
const port = (server.address() as any).port;

async function cycle() {
  await new Promise<void>(resolve => {
    const tcp = net.createConnection({ host: "127.0.0.1", port });
    tcp.on("error", () => resolve());
    tcp.on("connect", () => {
      const sec = tls.connect({ socket: tcp, rejectUnauthorized: false, servername: "localhost" });
      sec.on("error", () => { try { sec.destroy() } catch {}; resolve() });
      sec.on("secureConnect", () => sec.write("ping"));
      sec.on("data", () => { sec.destroy(); resolve() });
    });
  });
}

const t0 = Date.now();
for (let i = 0; i < N; i++) { await cycle(); if (INTERVAL_MS) await Bun.sleep(INTERVAL_MS); }
await Bun.sleep(200);
for (let i = 0; i < 5; i++) { Bun.gc(true); await Bun.sleep(100); }
const m = process.memoryUsage();
console.log(`done ${N} cycles in ${((Date.now()-t0)/1000).toFixed(1)}s  rss=${(m.rss/1024/1024).toFixed(1)}MB`);
server.close();
process.exit(0);

Safety

  1. upgradeTLS is client-side only. The existing early-return at the top of the function rejects server-side callers (this.isServer() check). Server-side TLS goes through Listener.connectInner / upgradeDuplexToTLS, both of which are untouched by this PR. The cache can't accidentally share a client SSL_CTX with a server SSL_CTX.
  2. Cache-reuse path skips SSL_CTX_set_tlsext_servername_callback / _arg. Those mutate the shared SSL_CTX, so we only call them on the first (cache-miss) creation. The SNI callback is only invoked server-side, so it can't fire against a reused SSL_CTX through this client-only path.
  3. Cache key is content-based (SSLConfig.contentHash(), already hashes every config field including cert/key/CA byte contents). Configs with different material map to different cache entries — verified with FinalizationRegistry instrumentation that 1000 upgrades with identical options populate exactly one cache bucket.
  4. Per-connection state stays per-connection. SSL*, peer cert, negotiated session ticket, BIO buffers — all created fresh by SSL_new(ssl_ctx) / us_socket_context_adopt_socket for every upgrade. Nothing connection-specific is shared.
  5. SSL_CTX_up_ref is the documented mechanism for sharing a SSL_CTX across multiple owners. Cache holds one ref; each wrapped us_socket_context_t that consumes the CTX holds its own ref; SSL_CTX_free decrements only. The cache's ref keeps the CTX alive across connection churn.
  6. Session cache sharing is a feature, not a side effect. Each SSL_CTX owns a client-side session cache; when the same SSL_CTX is reused across connections, subsequent handshakes can resume via cached session tickets. Pre-PR Bun gets zero session resumption across SDAM heartbeats (each heartbeat builds a fresh SSL_CTX with an empty cache); with this PR, heartbeats 2..N can abbreviate-handshake off heartbeat 1's tickets.

Prior art — who else shares SSL_CTX across many connections

Verified by source inspection, not by citing BoringSSL header comments in the abstract:

  • Chromium is the canonical precedent for a client stack that shares one SSL_CTX across all outbound TLS connections. net/socket/ssl_client_socket_impl.cc defines an SSLContext process-wide singleton that owns a single SSL_CTX (chromium/src/net/socket/ssl_client_socket_impl.cc, class SSLContext + ssl_ctx_.reset(SSL_CTX_new(...))); every SSLClientSocketImpl::Init() does ssl_.reset(SSL_new(context->ssl_ctx())) off that shared CTX. One SSL_CTX, N SSL*, process-lifetime — more aggressive sharing than what this PR does.
  • rust-openssl / boring / native-tls document the pattern explicitly. From openssl::ssl::SslContext: "Applications commonly configure a single SslContext that is shared by all of its SslStreams." SslConnector is #[derive(Clone)] over an internally-refcounted SslContext, so cloning a connector is SSL_CTX_up_ref. Same shape in boring and native-tls.
  • reqwest (reqwest::Client) — "The Client holds a connection pool internally… so it is advised that you create one and reuse it." The TLS connector (and thus SSL_CTX) is pinned for the Client's lifetime.
  • Go net/http — crypto/tls.Config: "A Config may be reused; the tls package will also not modify it." One *tls.Config on http.Transport, reused for every dial.

Node.js is a counter-example, not supporting evidence. I initially mis-claimed this PR "matches Node's SecureContext pattern". After re-reading the source: Node creates a fresh SecureContext per tls.connect() (lib/internal/tls/wrap.js:1740), marks it singleUse unless keepAlive is set (wrap.js:1734), and closes it synchronously on socket destroy (wrap.js:795-797). The _sessionCache on HTTPS Agent (https.js:465) caches session tickets for abbreviated handshakes, not SSL_CTX.

Why Node gets away with "new SSL_CTX per connection" and Bun doesn't

It's not BoringSSL vs OpenSSL — both are C code allocating through plain malloc, both fragment the allocator the same way per SSL_CTX_new/SSL_CTX_free pair. The difference is the cleanup path:

Node.jsBun (pre-PR)
Trigger to start cleanupsocket close eventsocket close event
Next hopsetImmediate(destroySSL) — next event-loop tickJSC finalize — whenever GC decides to walk
After that_destroySSL runs inline, calls context.close() → SSL_CTX_freeZig deinit → socket_context.deinit → nextTick(_deinit_ssl) → us_socket_context_free (refcount decrement)
Refs on SSL_CTX at close time1 (single TLSWrap)2 (tls wrapper + raw wrapper — upgradeTLS allocates both)
Worst-case latency: close → actual free~1 tickunbounded (GC-gated) × 2 (both wrappers must finalize)

Node's peak in-flight SSL_CTX count is ~1 under any workload. Bun's peak can be hundreds of SSL_CTXs while JSC GC catches up to the close events — and each in-flight SSL_CTX is ~50KB, so peak in-flight sets the allocator high-water-mark. On glibc ptmalloc that high-water-mark never comes down, which is the user-reported "RSS grows over days in Docker".

Bun's release path is not going to become synchronous (setImmediate-style) without a larger refactor that lifts SSL_CTX release off the JSC finalize queue. So the practical fix is to not depend on the release path: cache the SSL_CTX so there's effectively nothing to free per connection. That's this PR, and it's the Chromium approach.

Primary-source safety references for SSL_CTX_up_ref specifically

What changed

New in libusockets C API:

  • us_socket_wrap_with_tls_using_ssl_ctx(...) — wrap-with-tls variant that takes a caller-provided SSL_CTX* + SSL_CTX_up_refs it instead of SSL_CTX_new
  • us_socket_context_get_ssl_ctx(...) — retrieves SSL_CTX* from a wrapped context for cache storage

Refactor in openssl.c: original us_internal_ssl_socket_wrap_with_tls body becomes a shared _impl taking an optional shared_ssl_ctx; two public entry points dispatch into it. The reuse path skips SSL_CTX mutation (SNI callback setup).

Zig: SocketContext.getSSLCtx(), wrapTLSUsingSSLCtx on the socket handle, and the cache itself (ssl_ctx_cache, lookupCachedSSLCtx, storeCachedSSLCtx) + a two-line hook in upgradeTLS that picks the reuse path on cache hit and stashes the freshly-minted SSL_CTX on miss.

Related PRs

  • #26766 — wrapper-lifecycle fix (merged 2026-02-17). Addressed a separate layer of the same bug; this PR assumes #26766 is in place.
  • #29451 — JSRef wrapper-lifecycle refactor (merged 2026-04-18). Also complementary.

Test plan

test/regression/issue/12117.test.ts gets a second it() block: "should not grow RSS per upgrade when the TLS config is shared". It does 300 starttls cycles with the same options object and asserts RSS growth stays under 100MB. Verified locally:

  • Both tests pass with the fix — bun scripts/build.ts --profile=debug --asan=off test test/regression/issue/12117.test.ts → 2 pass, 0 fail in ~2.3s
  • New test fails without the fix (regression-test validity) — temporarily short-circuiting lookupCachedSSLCtx to return null (same HEAD, same binary, only that one branch swapped out) makes the new RSS test exceed the threshold / time out. The existing wrapper test still passes (unrelated code path). That means the new assertion genuinely exercises the cache.
  • Existing wrapper test still green — Alan Stott's original test continues to pass with the fix applied.
  • Allocation-volume claim verified — SSL_CTX_new call count drops from 1000 to 1 across a 1000-cycle run (verified via temporary counter instrumentation during development; counter code not included in the PR).
  • CI passes