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

net/http: leaking connections in Client #4049

Closed
gopherbot opened this issue Sep 6, 2012 · 13 comments
Closed

net/http: leaking connections in Client #4049

gopherbot opened this issue Sep 6, 2012 · 13 comments
Milestone

Comments

@gopherbot
Copy link

by thejuskrishna:

HTTP connection stays open even after the request is sent and also after closing it
forcefully. Feels like it is a bug.
@mrosset
Copy link
Contributor

mrosset commented Sep 8, 2012

Comment 1:

can you paste some code for this? I have seen this where a client is not reused over a
period of time it will leak connections.

@gopherbot
Copy link
Author

Comment 2 by ajdovahkiin:

thanks for the response mike. We are not reusing the client because we need to specify a
dynamic timeout. Within a day's time around 2000 open connections are piled up in
"ESTABLISHED" state.
Please find below the relevant section of the code.
func PostForm(url string, data url.Values, tracker string, timeout time.Duration) (body
[]byte, err error) {
    out.Printf("[%s] Firing a POST request to [%s] with data [%v]\n", tracker, url, data)
    var resp *http.Response
    client := http.Client{
        Transport: &http.Transport{
            Dial:              timeoutDialler(timeout, tracker),
            DisableKeepAlives: true,
        },
    }
    resp, err = client.PostForm(url, data)
    if err != nil {
        out.Printf("[%s] Error in POST request. Reason = [%s]", tracker, err)
        return
    }
    defer resp.Body.Close()
    body, err = ioutil.ReadAll(resp.Body)
    if err != nil {
        out.Printf("[%s] Error in fetching request body. Reason = [%s]", tracker, err)
        return
    }
    return
}
CALL (within a goroutine):
response, err = PostForm(url, postParms, tracker, time.Duration(210*time.Second))
Our app spawns several goroutines and each of them talks to multiple servers running on
different machines using this POST request. I have set the limit for max parallel
goroutines to 15. 
Please let me know if you need any more information or if there is a better way of doing
it.

@mrosset
Copy link
Contributor

mrosset commented Sep 10, 2012

Comment 3:

This is easy to fix, if you make your client a global var, this should solve your leaky
connection. also your notice an improvement in speed, since it will reuse connections,
even though you have set DisableKeepAlives guess to try and fix the leak?
from net/http client docs.
The Client's Transport typically has internal state (cached TCP connections), so Clients
should be reused instead of created as needed. Clients are safe for concurrent use by
multiple goroutines.

@gopherbot
Copy link
Author

Comment 4 by ajdovahkiin:

Thanks Mike. I will try reusing the client and let you know if it worked.
Is there a way to add a timeout if i declare a global var? Because in my case i need
different requests to timeout after different intervals of time.
Currently i am doing it this way whenever i create a new client object,
func timeoutDialler(timeout time.Duration, tracker string) func(net, addr string)
(client net.Conn, err error) {
    return func(netw, addr string) (net.Conn, error) {
        client, err := net.DialTimeout(netw, addr, time.Duration(30*time.Second))
        if err != nil {
            out.Printf("[%s] Failed to connect to [%s]. Timed out after 30 seconds\n", tracker, addr)
            return nil, err
        }
        client.SetDeadline(time.Now().Add(timeout))
        return client, nil
    }
}

@mrosset
Copy link
Contributor

mrosset commented Sep 10, 2012

Comment 5:

One thing I should have mentioned, is you do not have to make client a global var, you
can pass it to PostForm as a pointer.
As for timing out, I've not had to do this my self. But one method might be to create
your own client struct and embed http.Client. then say attach a Timeout method that
changes the Dial function. however you would have to provide some synchronization with
this method.

@gopherbot
Copy link
Author

Comment 6 by entcrd:

I have the same problem , timeout checker + Post - does not close connections .

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 7:

I am not sure this is a leak. It might just be a delay.

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 8:

Labels changed: added go1.1.

@gopherbot
Copy link
Author

Comment 9 by temotor:

This code may help to run more tests. http://play.golang.org/p/w5JMXiGLCT
If client does  23: tcp_conn.SetLinger(0)  then connections are closed right away. If
that line is commented, connections hang in TIME_WAIT.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 5, 2012

Comment 10:

It's not clear to me what this bug is about, or if there's actually a problem here or
people are just talking about TCP's normal behavior.
There have been a number of DialTimeout and HTTP changes in the past few months.
I'm setting this bug to WaitingForReply for now.  Please reply with a summary of the
problem, example code showing the problem, and which version of Go and operating system
you're using.
If I don't get a reply, I'll close this assuming it's an old dup.

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

Comment 11 by thejuskrishna:

I think this issue got resolved in the new build.

@gopherbot
Copy link
Author

Comment 12 by ajdovahkiin:

Sorry for not updating. This issue is fixed in 1.0.3

@bradfitz
Copy link
Contributor

bradfitz commented Dec 5, 2012

Comment 13:

Okay, cool.  I don't remember what would've fixed it in 1.0.3, but happy to close it.

Status changed to Retracted.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants