Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: net/http transport goroutines leak #317

Closed
wants to merge 3 commits into from

Conversation

laoleesch
Copy link

Hi! For each transport flush there hung a couple "zombie"-routines
net/http.(*persistConn).readLoop
net/http.(*persistConn).writeLoop

I suppose it because there is no response.Body.Close() in transport cycles. This not only leads to a zombie-routines but also because of the lack of re-using connections to the loss of performance (no keep-alive).

net/http/#Client.Do

If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close. If the Body is not both read to EOF and closed, the Client's underlying RoundTripper (typically Transport) may not be able to re-use a persistent TCP connection to the server for a subsequent "keep-alive" request.

It solves resource leak issue in my cases.

Thanks!

@laoleesch
Copy link
Author

Issue #316

Copy link
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to drain the response body, could use io.CopyN (see google/go-github#317 (comment)).
Not sure this is still relevant in recent Go versions, the docs suggest so.

https://golang.org/pkg/net/http/#Client.Do

If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close. If the Body is not both read to EOF and closed, the Client's underlying RoundTripper (typically Transport) may not be able to re-use a persistent TCP connection to the server for a subsequent "keep-alive" request.

https://golang.org/pkg/net/http/#Response

// [...] It is the caller's responsibility to
// close Body. The default HTTP client's Transport may not
// reuse HTTP/1.x "keep-alive" TCP connections if the Body is
// not read to completion and closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants