Conversation
On Linux the client TLS session cache kept exactly one SSL_SESSION per SNI host. For TLS 1.3 that is wrong in two compounding ways. An OpenSSL server sends two NewSessionTickets after an initial handshake, so new_session_cb fires twice. The old TryAddSession handled the second by evicting the first and calling SSL_CTX_remove_session on it, and remove_session_lock sets not_resumable = 1 unconditionally. The discarded ticket was therefore invalidated, not merely dropped, on every connection. Concurrent connections then contend for the single survivor. SSL_set_session shares the pointer rather than copying, so when the first of several concurrent handshakes completes, tls_finish_handshake removes that shared session and marks it not_resumable; every other connection holding it falls back to a full handshake. Cache TLS 1.3 tickets in a small per-host pool so concurrent handshakes can each take a distinct one. TLS 1.2 still keeps a single entry, which is both sufficient and all OpenSSL ever provides, since it skips new_session_cb on resumed TLS 1.2 handshakes. Add a CryptoNative_SslSessionUpRef shim so TrySetSession can take a reference under the managed lock, release the lock, and only then call SSL_set_session. RemoveSession frees outside that lock, so relying on the lock alone to keep the session alive across the call was unsound. Measured with the concurrent handshake benchmark from dotnet/performance#5314 over loopback sockets, TLS 1.3 with resumption: 14% faster at 64 concurrent connections and 12% at 128. TLS 1.2 and the non-resuming cases are unchanged; low concurrency costs 2-3%.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add focused TLS 1.3 pool tests and update the stale cache invariant comment.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Updates Unix OpenSSL TLS session caching to pool multiple TLS 1.3 tickets per host, improving concurrent resumption while preserving TLS 1.2 behavior.
Changes:
- Adds native and managed
SSL_SESSION_up_refinterop. - Implements bounded per-host session pools with eviction.
- Detects negotiated TLS versions when caching sessions.
| File | Summary |
|---|---|
src/native/libs/System.Security.Cryptography.Native/pal_ssl.h |
Declares session reference incrementing. |
src/native/libs/System.Security.Cryptography.Native/pal_ssl.c |
Implements the native wrapper. |
src/native/libs/System.Security.Cryptography.Native/opensslshim.h |
Registers the OpenSSL symbol. |
src/native/libs/System.Security.Cryptography.Native/entrypoints.c |
Exports the native entry point. |
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs |
Manages pooled sessions, eviction, and references. Moderate (3 votes): add focused concurrency and eviction coverage. Nit (1 vote): update the stale single-session cache comment. |
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs |
Adds managed interop bindings. |
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs |
Classifies sessions by negotiated TLS version. |
| } | ||
|
|
||
| internal unsafe bool TryAddSession(byte* namePtr, IntPtr session) | ||
| internal unsafe bool TryAddSession(byte* namePtr, IntPtr session, bool isTls13) |
This was referenced Sep 20, 2026
rzikm
self-requested a review
September 21, 2026 14:29
rzikm
approved these changes
Sep 21, 2026
Comment on lines
+226
to
230
| if (!_sslSessions.TryGetValue(targetName, out List<CachedSession>? sessions)) | ||
| { | ||
| if (!_sslSessions.TryAdd(targetName, session)) | ||
| { | ||
| // session to this target host exists, replace it | ||
| _sslSessions.Remove(targetName, out oldSession); | ||
| bool added = _sslSessions.TryAdd(targetName, session); | ||
| Debug.Assert(added); | ||
| } | ||
| sessions = new List<CachedSession>(); | ||
| _sslSessions[targetName] = sessions; | ||
| } |
Member
There was a problem hiding this comment.
Since we are touching this, we can switch to https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.collectionsmarshal.getvaluereforadddefault?view=net-10.0#system-runtime-interopservices-collectionsmarshal-getvaluereforadddefault-2(system-collections-generic-dictionary((-0-1))-0-system-boolean@)
This branch has not been deployed
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
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.
On Linux the client TLS session cache kept exactly one SSL_SESSION per SNI host. For TLS 1.3 that is wrong in two compounding ways. We can get unnecessary tuen when server sends more tickets and we also have problem with concurrency as we cannot use the same ticket twice for TLS 1.3. The cache design was done for TLS 1.2 where using same ticket over and over again is fine. Also we have possible lock ordering problem.
Measured with the concurrent handshake benchmark from dotnet/performance#5314 over loopback sockets, TLS 1.3 with resumption: 14% faster at 64 concurrent connections and 12% at 128. TLS 1.2 and the non-resuming cases are unchanged; low concurrency costs 2-3%.
Note that this really depends on machine and timing. The problem is nearly invisible when using Memory stream and everything is super fast. But that is not the real world scenario.
This PR separates the behavior so we can have multiple (up to 8) tickets so we have better chance of resumption during parallel processing. We would hold eactly one ticket for cases when site is visited once and never again after e.g. crawlers.
Effect of the fix
Measured with
SslStreamConcurrencyTests.ConcurrentHandshakefromdotnet/performance#5314, which performs concurrent TLS handshakes over loopback
sockets. Both runtimes were driven by BenchmarkDotNet's CoreRun toolchain in a
single run on the same machine, with the unmodified build as the baseline, so
the ratios are directly comparable. Linux x64, OpenSSL 3.0.13, RSA-2048 server
certificate. Ratio below 1.00 means the fix is faster.
TLS 1.3 with resumption enabled
14% faster at 64 concurrent connections and 12% at 128.
Cases the change should not affect
Cost
At low concurrency TLS 1.3 resumption is 2–3% slower (1.03 at N=1, 1.02 at
N=8) and allocates about 1% more (12.44 KB vs 12.33 KB at N=64). This is the
extra
SSL_SESSION_up_refcall and the list lookup replacing a dictionarylookup.
Eviction and cache cleanup
OpenSSL continues to enforce its own global cap of
DefaultTlsCacheSizeClientsessions across all hostnames, unchanged by this PR. When the cache is full
SSL_CTX_add_sessionevicts fromctx->session_cache_tailuntil it is backunder the limit, and since
SSL_SESSION_list_addkeeps the list ordered byeffective expiry rather than by insertion or use, the victim is always the
session nearest to expiring — effectively oldest-first, so idle hosts shed
entries before active ones. Every removal path invokes
remove_session_cb,which is how the managed dictionary stays in sync and how its size stays
transitively bounded by that same cap: the callback finds the entry by the
hostname stashed on the session and by pointer identity, then drops exactly one
reference. OpenSSL raises it even when the session was not in its own hash, so
the removal is guarded to release once per cached reference.