TLS session resumption for resolved suspended routes; reclaim expired IP-affinity entries

[kriszyp]

⊙ Problem

resolveConnection() with terminateTls gave every resolved connection a brand-new rustls ServerConfig from a throwaway cache, so each resolution re-parsed its cert, allocated a fresh session store (preallocated for 1024 entries, ~125 KB) and a fresh ticketer — and no client of a resolved route could ever resume a TLS session. Separately, CI ran cargo clippy --all-targets without -D warnings, and main carried 8 dead-code warnings. One of them, UdsBalancer::evict_affinity, is a background task CLAUDE.md says start() spawns, but nothing ever called it: with ipAffinity on, every distinct client IP stayed in the affinity map until the next route-table reload.

❓ Your call: is the requirement right? As specified, yes for both halves. The affinity-eviction wiring goes past "delete dead helpers": deleting evict_affinity would have silenced the only signal of an unbounded map. It is its own commit (6af5459) and can be dropped if you would rather track it separately.

💡 Solution

• Resolved routes now take their ServerConfig from the proxy's own TlsConfigCache, so resolutions of one cert share a config with each other — and with a table route carrying the same material — and therefore share its ticketer and session store. End to end, a second connection now resumes under TLS 1.2 and 1.3; both failed on base.
• Lifetime. No route table holds a resolved config, so it lives by refcount alone: kept while a connection holds it, otherwise dropped at the next route-table build's clear_used, or when an insert pushes the cache past evict_at = max(256, 2 × configs still referenced). Resolutions go through get_or_build_unmarked and write no marks, so the mark set still means "requested by the build in progress".
• Affinity. start() spawns evict_affinity_periodically: every 60 s it reloads the live table and runs evict_affinity on the blocking pool for each affinity balancer, collected from the route actually installed (h2 mirrors and carried-forward routes included). It removes only entries pick() already treats as expired, so routing is unchanged.
• Dead code removed; CI runs cargo clippy --all-targets -- -D warnings.

⚠️ Look hardest: evict_unreferenced — every cache eviction decision is Arc::strong_count(config) > 1. The claim is that anything still in use has a second owner — a route table, or a live connection (the TlsAcceptor in proxy_conn.rs:242 lives through proxy_via_tls) — so eviction can only cost a future resumption, never a live connection.

⚖️ Alternatives

• Planning review returned better-alternative-exists, adopted. My plan kept idle resolved configs until the next reload; the reviewer showed a proxy that never reloads would then keep one config per distinct cert ever resolved. Adopted its pressure-triggered refcount sweep on insert instead.
• Separate resolve-only cache: rejected — a cert shared by a table route and a resolution would get two ticketers, so a ticket from one path would fail on the other.
• API change so JS passes a cert/route handle instead of PEM: new API surface to save one SHA-256 over the PEM per resolution.
• Process-wide cache: orthogonal (it would also survive listener recreation); not done here.

❓ Your call: idle resolved configs are dropped at every route-table build, including one updateConfig then rejects, so each returning client of an idle resolved route does one full handshake per reload. Keeping them would need a second mark set that a failed build's marks can't be confused with; I judged that not worth it. Reversible.

❓ Your call: a resolution with the same cert, key, client-auth CA and requirement, and ALPN as a table route shares that route's config and ticketer. That is safe only because the cache key covers every input a resumed session may skip; a future ServerConfig input not added to the key would break it. Easy to split later.

❓ Your call: MIN_EVICT_AT = 256 caps idle resolved configs at roughly 256 × ~125 KB ≈ 32 MB when few configs are referenced; above that the threshold tracks 2× the referenced set. Lower saves memory, higher keeps resumption for more idle tenants.

❓ Your call: affinity eviction is a once-per-minute DashMap::retain on the blocking pool; a pick() that hashes to the shard under scan waits for that shard. Measured on 1 M entries (release build, unthrottled 20-core dev box, 128 shards): 14 ms total when nothing expires, 129 ms when everything does (~1 ms per shard). Evicting inside pick() instead would run the same full scan on a tokio worker in one connection's path, since a DashMap has no cheap way to find expired entries.

🔧 Changes

• src/tls.rs — get_or_build (marks, for table builds) and get_or_build_unmarked (for resolutions) over a shared lookup_or_build; evict_unreferenced, used by clear_used and by a miss past evict_at, then shrinks the map to evict_at so later scans don't walk an allocation left over from a large drained set.
• src/suspended.rs — build_resolved_route takes the proxy's &Mutex<TlsConfigCache> and locks it only for terminating TLS; unused pending_count removed.
• src/proxy.rs — passes self.tls_cache to the resolver; spawns the affinity-eviction loop in start(); uses LiveRouteTable::new instead of inlining it; holds the runtime as _rt: Runtime (tokio's Runtime is Send + Sync, so the Mutex<Option<..>> and its comment were stale).
• src/router.rs — RouteTable::affinity_balancers, collected beside monitored_balancers; evict_affinity_periodically; test fixtures made pub(crate) for the other modules' tests.
• src/balancer.rs — has_affinity; evict_affinity's doc corrected (it checks age only, not active connections); unused connection_counts removed.
• src/http_proxy.rs — removed unused is_connection_close, status_code, with_connection_keep_alive, insert_x_forwarded_for (XFF is a HeaderRewrite applied by rewrite_request_head).
• .github/workflows/CI.yml — -D warnings.
• CLAUDE.md — TlsConfigCache section: resolved-route lifetime and the eviction bound.

✅ Verification

• End to end (Node → napi → suspended registry → real TLS handshakes): new suite in __test__/session-resumption.spec.ts — a suspended route with no cert of its own, auto-resolved with one; the second connection offering the first's session resumes under TLS 1.3 and 1.2, with two resolutions observed. Both cases failed on base (reused === false). The same file's teardown now tolerates a failed setup. __test__/suspended.spec.ts only updates a walkthrough comment.
• Rust: suspended.rs tests — two resolutions share one config; mTLS required/optional and h2 key separate configs; a resolution shares a table route's config; a held config survives a reload and an idle one doesn't; a failed resolution leaves the cache usable. tls.rs bound test, which fails when the pressure sweep is disabled. router.rs — affinity collection incl. h2 mirror and carry-forward; the eviction loop follows a hot-swap and exits on shutdown. balancer.rs — TTL-only eviction.
• cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean, cargo test 153/153, npm run build:debug && npm test 153/153.
• Not covered by a test: the spawn line in start() itself (no JS-observable effect); checked by inspection.

_{🤖 Generated by Claude Opus 5.5 (Anthropic Claude Code); posted via @kriszyp.}

Related PRs: none found
Complexity: medium

Origin — the dispatch brief this PR was written from

Reuse the proxy TlsConfigCache for resolved routes; clippy -D warnings

_{Dispatch: task symphony-resolved-route-tls-cache-and-clippy · queued by unknown · ran by claude/opus/xhigh · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer,cursor-kimi,cursor-muse; rounds=2; full=1 @ fb80fac}

_{Review-Attention: read ~6m (decisions: resolved-config-lifetime, shared-table-and-resolve-cache, affinity-eviction-shard-locks, do-less-alternative) @ fb80fac}

[gemini-code-assist]

Code Review

This pull request integrates the proxy's TlsConfigCache with suspended route resolutions (resolveConnection), enabling TLS session resumption for resolved routes while bounding memory growth via a new cache eviction strategy for unreferenced configurations. It also introduces periodic IP affinity eviction for UDS balancers to prevent memory leaks from expired affinity entries. Feedback on these changes suggests deduplicating the collected affinity_balancers to avoid redundant eviction passes, and properly handling the join handle of spawn_blocking during eviction to ensure background panics are not silently swallowed.