handshake: propagate leader load/obtain outcome to all waiters#387
Merged
mholt merged 1 commit intoJun 9, 2026
Merged
Conversation
Zenexer
commented
Jun 8, 2026
Zenexer
force-pushed
the
np/waiter-outcome-propagation-upstream
branch
from
June 8, 2026 14:06
bd1e865 to
7898be0
Compare
When the goroutine leading a certificate load/obtain for a name fails (e.g. an on-demand permission denial) or succeeds but the cert is evicted from the cache before waiters re-look it up, waiters recursively re-enter the wait queue instead of receiving the leader's outcome. Under sustained handshake load for uncertificated names, these recursing goroutines accumulate without bound (observed at 30k+ goroutines, recursion depth 48), each pinning live TLS handshake state — multi-GB memory growth within minutes. Propagate the leader's returned certificate or error to all waiters via the existing certLoadWaiter fields (previously populated only on the external-cert-manager path). Waiters now return the leader's outcome directly; the recursive fallback remains only for the rare empty-cert+nil-error case.
Zenexer
force-pushed
the
np/waiter-outcome-propagation-upstream
branch
from
June 8, 2026 14:10
7898be0 to
0c049a3
Compare
mholt
approved these changes
Jun 9, 2026
mholt
left a comment
mholt
left a comment
Member
There was a problem hiding this comment.
Thanks, this seems correct, I believe.
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
2 participants
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.
getCertDuringHandshakecoordinates concurrent handshakes for the same name throughcertLoadWaitChans. If multiple concurrent handshakes occur, they race; the winner loads or obtains a cert, while others wait.However, the winner's result is only correctly propagated to waiters on the external-cert-manager path (#369). On every other path (on-demand permission denial, a failed obtain, or a successful load whose cert is evicted from the cache before waiters re-check it), losers receive nothing and recursively re-enter the wait queue behind the next winner.
Under sustained handshake load for names that cannot get certificates (e.g. on-demand TLS receiving scanner traffic, or an
askendpoint rejecting/throttling), these recursing goroutines accumulate.This leak was observed in production on caddy 2.11.x, on-demand TLS, ~36 req/s:
Ideally, ask endpoints should be fast, so this should be a non-issue; in practice, that isn't always the case.
This was debugged and patched largely by Claude, Opus 4.8. I monitored what it was doing and reworked some of its wording, but it seems to have correctly identified the issue without much assistance.