gorequest icon indicating copy to clipboard operation
gorequest copied to clipboard

File Descriptor leak

Open slava-vishnyakov opened this issue 8 years ago • 15 comments

I have a code like this:

    func testRequest() {
        gorequest.New().Get("http://www.google.com").End()
    }

    for {
        go testRequest()
        time.Sleep(1 * time.Second)
        fmt.Println("Request done, open files =", countOpenFiles())
    }

The real code is a bit more complicated, but anyways..

It seems to be in line with what docs recommend, but it actually leaks file descriptors:

    for {
        go testRequest()
        time.Sleep(1 * time.Second)
        fmt.Println("Request done, open files =", countOpenFiles())
    }

the output that I get is:

Request done, open files = 15
Request done, open files = 17
Request done, open files = 19
Request done, open files = 21
Request done, open files = 23
Request done, open files = 25
Request done, open files = 27

Ok, let's change it a bit (which is not documented, but ok):

gorequest.New().Get("http://www.google.com").Set("Connection", "close").End()

This time it's better:

Request done, open files = 14 Request done, open files = 15 Request done, open files = 16 Request done, open files = 17 Request done, open files = 18 Request done, open files = 19 Request done, open files = 20

But it still leaks.

  1. Maybe the documentation should be explicit about the need to set .Set("Connection", "close")
  2. How to get rid of second leak? I can't reuse the connection since that's going in different goroutines

The full code is here: https://git.io/vaDBp

slava-vishnyakov avatar Mar 21 '16 12:03 slava-vishnyakov

same problem here, request in goroutine caused fd count increasing

freeznet avatar Mar 25 '16 08:03 freeznet

This was happening to me as well. I wrote a pass through service using echo that posted data to a partner API. Eventually I got this: echo => panic recover\n runtime error: invalid memory address or nil pointer dereference that was caused by this: [http] 2016/03/24 23:39:30 Error: read tcp x.x.x.x:xxx->y.y.y.y:443: read: connection reset by peer. Every message created a new connection and held it open until timeout.

Set("Connection", "close") solves my issue though.

The other open connections look like this: server 5367 user 25u IPv6 0x2e646f7ab8dde695 0t0 TCP localhost:48599->localhost:64978 (ESTABLISHED)

These timed out on my box, TBH I need to look more into this to figure out whether these matter.

Thanks for your post.

dustinevan avatar Apr 01 '16 22:04 dustinevan

@slava-vishnyakov
Can you print a message before func testRequest return. If the request never finish and never return with timeout, the fd count will grow.

roxma avatar May 18 '16 09:05 roxma

Well, I got the same problem here

roxma avatar May 18 '16 09:05 roxma

My solution was to go back the the net/http library. The growing file descriptor count issue can be fixed by disabling keep-alives, but the panic on connection reset makes this otherwise useful lib unstable imo.

dustinevan avatar May 18 '16 10:05 dustinevan

@dustinevan agreed,

request = gorequest.New()
request.Transport.DisableKeepAlives = true

work for me

roxma avatar May 18 '16 10:05 roxma

@dustinevan I don't understand how to make the panic hapen, could you describe it with more detail ?

roxma avatar May 18 '16 10:05 roxma

Say you have 100k requests to send, all scheduled in different goroutines, any library based on net/http will attempt to open connections and send the requests independently. This means--depending on how long it takes to establish a connection, and depending on how long it takes for the request to get a response, and depending how long it takes to schedule the goroutines--that in worse case you'll attempt to open 100k connections with a single server. If you overwhelm a server in this way, some well written servers will start resetting your connections--essentially hanging up the phone on you. If you receive a connection reset, the proper thing to do as a client is to wait, and then resend the request. net/http properly recovers from this situation to retry your requests, but this gorequest library panics instead. I guess you could write some recovery code to deal with this situation, but my solution was to not use the library.

That being said, none of this matters if you're sending a request here and there. This is only important in high traffic situations.

dustinevan avatar May 18 '16 10:05 dustinevan

@dustinevan Thanks, so tcp reset is a rare case with low traffic. I'll try figuring out how to fix this.

roxma avatar May 18 '16 14:05 roxma

@dustinevan I send thounds of requests to a server at the same time in order to get the RST response, instead of panic, I get an err [Get http://www.baidu.com: read tcp 10.104.67.229:44865->14.215.177.38:80: read: connection reset by peer]

This is the code:

package main

import (
    "time"
    "fmt"
    "sync"
    "github.com/parnurzeal/gorequest"
)

var wg sync.WaitGroup

func testRequest() {
    r := gorequest.New().Timeout(time.Millisecond*100).Get("http://www.baidu.com")
    _,_,err := r.End()
    if err != nil {
        fmt.Println(err)
    }
    wg.Done()
}

func main() {
    for i:=0; i<1000; i=i+1{
        wg.Add(1)
        go testRequest()
    }
    wg.Wait()
}

roxma avatar May 18 '16 16:05 roxma

Nice! Looks like I need to look deeper into my error then. Good to know!

dustinevan avatar May 18 '16 20:05 dustinevan

I recently ran into this issue while making requests with the stdlib. The problem arises from initialising a new http.Transport{} for every request. http.Transport{} is meant to be initialised once and then reused.

From the doc:

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.

knadh avatar Jul 12 '16 08:07 knadh

Is there any decision has been made. What is the best way to do concurrent request by reusing same TCP connection ?

sapamja avatar Dec 19 '16 02:12 sapamja

Does this problem also occur if you reuse *gorequest.SuperAgent ?

dvic avatar Oct 11 '17 15:10 dvic

I can confirm that the example reported by @roxma does not exhibit the error when using a fixed http.Client and http.Transport. Should we introduce a gorequest.NewE(client *http.Client, transport *http.Transport) method where one can provide the client and transport?

dvic avatar Oct 11 '17 22:10 dvic