net/http: leaking connections in Client

[gopherbot]

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]

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.

[mrosset]

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]

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.

[gopherbot]

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]

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.

[mrosset]

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]

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
    }
}

[gopherbot]

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]

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.

[mrosset]

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]

Comment 6 by entcrd:

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

[gopherbot]

Comment 6 by entcrd:

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

[rsc]

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]

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]

Comment 8:

Labels changed: added go1.1.

[rsc]

Comment 8:

Labels changed: added go1.1.

[gopherbot]

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.

[gopherbot]

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]

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.

[bradfitz]

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]

Comment 11 by thejuskrishna:

I think this issue got resolved in the new build.

[gopherbot]

Comment 11 by thejuskrishna:

I think this issue got resolved in the new build.

[gopherbot]

Comment 12 by ajdovahkiin:

Sorry for not updating. This issue is fixed in 1.0.3

[gopherbot]

Comment 12 by ajdovahkiin:

Sorry for not updating. This issue is fixed in 1.0.3

[bradfitz]

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.

[bradfitz]

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.