Repository navigation
Conversation
EnableConnectionReuse=false builds a client outside the pool for a single request and then drops it. Nothing closes what that client opened: the roundTripper holding the connection goes out of scope still connected, with no reference left anywhere to close it from. So the option that exists to avoid holding connections open held them open harder than leaving reuse on -- reuse at least reaches the same connection again, where this stranded a fresh one per request. Measured before this change, ten requests with reuse off left eight sockets behind, against nine with it on. Close them at the end of Do() when the client came from outside the pool. That is safe precisely because it did: no other request shares it, and Do() has already read the body to completion by then, so nothing is still reading. The pooled path is untouched, where closing would pull connections out from under concurrent callers. Twelve requests with reuse off now leave nothing behind, and a caller that wants a fresh TLS connection per request gets one for the cost of the handshake.
|
Thanks for tracking this down - we just hit the exact same failure mode in production: fd-per-request growth with One related spot that has the same leak shape: |
smeinecke
added a commit
to smeinecke/cycletls_python
that referenced
this pull request
Oct 26, 2026
Port of Danny-Dasilva/CycleTLS#426. With enable_connection_reuse=false, getOrCreateClient builds a fresh client outside the pool and Do() drops it without closing what it opened - dialTLS caches the live conn into rt.cachedConnections and nothing ever frees it. On HTTP/2 (the common ALPN outcome) the ClientConn read/write loops pin the socket open, so every request leaked a socket + 2 goroutines until EMFILE. HTTP/1 was already covered by DisableKeepAlives; h2 has no such knob. - CycleTLS.Do(): close all connections on the roundTripper when the client came from outside the pool. Safe: nothing shares it and the body is fully read by then. Covers every FFI entry point. - dispatcherAsync: non-pooled clients now close all conns instead of keeping the just-served hostPort conn cached on a dropped client. - roundTripper.CloseIdleConnections: also drain idle conns inside cached transports (graceful h2 GOAWAY) and drop transport entries that have no cached conn (http-scheme path was never cleaned). Regression test drives 8 reuse-off requests against a local h2 server and asserts goroutine/fd counts stay flat; fails on the unfixed code (+24 goroutines, +16 fds).
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.
Problem
EnableConnectionReuse: falseleaks a socket per request — and leaks harder than leaving reuse on.With reuse disabled,
getOrCreateClientskips the pool and callscreateNewClient, so eachDo()builds its own client and roundTripper and then drops it. Nothing closes what it opened. Reuse-on at least keeps reaching the same connection; reuse-off strands a fresh one every request.Measured on
v1.0.30, 10 requests to 10 hosts, counting the process's own sockets:Change
Close the client's connections at the end of
Do()when the client came from outside the pool.