[Bug]: This may be a bug related to memory leaks

biaosheng commented 1 year ago

TLS client version

v1.4

System information

Window 11

Issue description

Hi, brother

When I use multi coroutine testing, the memory keeps increasing! This may be a bug related to memory leaks

Here is my test code:

Steps to reproduce / Code Sample

package main

import (
    "fmt"
    http "github.com/bogdanfinn/fhttp"
    tls_client "github.com/bogdanfinn/tls-client"
    "io"
    "log"
    "time"
)

func main() {
    for i := 0; i < 500; i++ {
        go request()
    }
    time.Sleep(time.Hour)
}
func request() {
    for {
        test()
        time.Sleep(time.Second * 5)
    }

}
func test() {
    jar := tls_client.NewCookieJar()
    options := []tls_client.HttpClientOption{
        tls_client.WithTimeoutSeconds(30),
        tls_client.WithClientProfile(tls_client.Chrome_105),
        tls_client.WithNotFollowRedirects(),
        tls_client.WithCookieJar(jar), // create cookieJar instance and pass it as argument
    }
    client, err := tls_client.NewHttpClient(tls_client.NewNoopLogger(), options...)
    if err != nil {
        log.Println(err)
        return
    }
    //client.SetProxy("http://127.0.0.1:8888")
    req, err := http.NewRequest(http.MethodGet, "https://www.baidu.com", nil)
    if err != nil {
        log.Println(err)
        return
    }
    req.Header = http.Header{
        "accept":          {"*/*"},
        "accept-language": {"de-DE,de;q=0.9,en-US;q=0.8,en;q=0.7"},
        "user-agent":      {"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36"},
        http.HeaderOrderKey: {
            "accept",
            "accept-language",
            "user-agent",
        },
    }

    resp, err := client.Do(req)
    if err != nil {
        log.Println(err)
        return
    }
    defer resp.Body.Close()
    log.Println(fmt.Sprintf("status code: %d", resp.StatusCode))
    io.ReadAll(resp.Body)

}

bogdanfinn commented 1 year ago

Thank you @biaosheng for reporting this issue. i will take a look on that. also @justhyped already supplied a lot of information to me regarding this and maybe a possible fix. we will investigate and try to reproduce / fix this.

I have the information from others that this issue started happening since 1.3.9. can you confirm that you dont have issues when using 1.3.8 or older?

bogdanfinn commented 1 year ago

@biaosheng

i can't really reproduce the memory leak issue ... or i dont see it. but i will try to explain what i see and why i think i see it:

When i run the example code from this issue i see increasing memory but after a few minutes the memory consumption levels off ...

When we look at the example code to reproduce the issue we see that a new client is created on each request per go routine. Means 500 clients every 5 seconds. This consumes lots of memory of course. The Garbage Collection of Go should handle that but this takes a while (The GC cycles are running in bigger intervals afaik) therefore we have to run the application some time to see that the memory consumption levels off .. Run the code for 15-20 minutes and you should see that the memory will be stable.

We can try to optimize the code a bit to create a client per goroutine but reuse the client instance on every request. With that we are not allocating new memory per request for a whole new client but only for the request itself. This will limit the total memory consumption a lot:

package main

import (
    "fmt"
    http "github.com/bogdanfinn/fhttp"
    tls_client "github.com/bogdanfinn/tls-client"
    "io"
    "log"
    "time"
)

func main() {
    for i := 0; i < 500; i++ {
        go request()
    }
    time.Sleep(time.Hour)
}

func request() {
    jar := tls_client.NewCookieJar()
    options := []tls_client.HttpClientOption{
        tls_client.WithTimeoutSeconds(30),
        tls_client.WithClientProfile(tls_client.Chrome_105),
        tls_client.WithNotFollowRedirects(),
        tls_client.WithCookieJar(jar), // create cookieJar instance and pass it as argument
    }
    client, err := tls_client.NewHttpClient(tls_client.NewNoopLogger(), options...)
    if err != nil {
        log.Println(err)
        return
    }

    for {
// pass the created client each iteration instead of recreating it
        test(client)
        time.Sleep(time.Second * 5)
    }

}
func test(client tls_client.HttpClient) {
    //client.SetProxy("http://127.0.0.1:8888")
    req, err := http.NewRequest(http.MethodGet, "https://www.baidu.com", nil)
    if err != nil {
        log.Println(err)
        return
    }
    req.Header = http.Header{
        "accept":          {"*/*"},
        "accept-language": {"de-DE,de;q=0.9,en-US;q=0.8,en;q=0.7"},
        "user-agent":      {"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36"},
        http.HeaderOrderKey: {
            "accept",
            "accept-language",
            "user-agent",
        },
    }

    resp, err := client.Do(req)
    if err != nil {
        log.Println(err)
        return
    }
    defer resp.Body.Close()
    log.Println(fmt.Sprintf("status code: %d", resp.StatusCode))
    io.ReadAll(resp.Body)
}

With that adjustment you should have way less memory consumption as this code is a bit more optimized.

Here is some reference implementation of your code with the std lib http client where we see also a huge memory consumption if you do not reuse the client and create a new one on each single request:

import (
    "fmt"
    "io"
    "log"
    "net/http"
    "time"
)

func main() {
    for i := 0; i < 500; i++ {
        go request()
    }
    time.Sleep(time.Hour)
}
func request() {
    for {
        test()
        time.Sleep(time.Second * 5)
    }

}

func test() {
    req, err := http.NewRequest(http.MethodGet, "https://www.baidu.com", nil)
    if err != nil {
        log.Println(err)
        return
    }
    req.Header = http.Header{
        "accept":          {"*/*"},
        "accept-language": {"de-DE,de;q=0.9,en-US;q=0.8,en;q=0.7"},
        "user-agent":      {"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36"},
    }

    client := &http.Client{
        Transport: &http.Transport{
            Proxy: http.ProxyFromEnvironment,
            DialContext: defaultTransportDialContext(&net.Dialer{
                Timeout:   30 * time.Second,
                KeepAlive: 30 * time.Second,
            }),
            ForceAttemptHTTP2:     true,
            MaxIdleConns:          100,
            IdleConnTimeout:       90 * time.Second,
            TLSHandshakeTimeout:   10 * time.Second,
            ExpectContinueTimeout: 1 * time.Second,
        },
    }

    resp, err := client.Do(req)
    if err != nil {
        log.Println(err)
        return
    }
    defer resp.Body.Close()
    log.Println(fmt.Sprintf("status code: %d", resp.StatusCode))
    io.ReadAll(resp.Body)
}

func defaultTransportDialContext(dialer *net.Dialer) func(context.Context, string, string) (net.Conn, error) {
    return dialer.DialContext
}

biaosheng commented 1 year ago

thanks,I will testing again!

bogdanfinn commented 11 months ago

Closed due to inactivity

bogdanfinn / tls-client