bogdanfinn / tls-client

net/http.Client like HTTP Client with options to select specific client TLS Fingerprints to use for requests.
BSD 4-Clause "Original" or "Old" License
670 stars 133 forks source link

Concurrent operation error #53

Closed qizhoumeng closed 1 year ago

qizhoumeng commented 1 year ago

Running requests under 100 concurrent conditions often results in errors。How to solve it

runtime error: invalid memory address or nil pointer dereference

bogdanfinn commented 1 year ago

@qizhoumeng do you maybe have more from the stacktrace? maybe the line number of the file where the nil pointer dereference happens?

internpoon commented 1 year ago

happen to me as well. just a little under 100 go routine. sometime even 2-3 concurrent req

panic: runtime error: invalid memory address or nil pointer dereference

bogdanfinn commented 1 year ago

@internpoon do you have maybe more of the stacktrace for me? or even better an example implementation to reproduce the issue?

internpoon commented 1 year ago

I just did a very simple request with proxy

CleanShot 2023-06-16 at 10 23 06@2x CleanShot 2023-06-16 at 10 17 35@2x
bogdanfinn commented 1 year ago

@internpoon

First of all the root cause: When setting a proxy on a client instance this creates a complete new underlaying transport struct as we need to connect to the new proxy. In combination with the fact that you are sharing one client instance across multiple go routines this is running into a race condition. What you are by accident doing is changing the proxy for every go routine at once.

You can get around this when you: a) either synchronize the proxy setting across all go routines which are sharing the same client and make sure no go routine is doing something with the transport while its being recreated due to the proxy switch b) or you create a separate client per go routine.

Next to it you should read about using the defer statement inside a for loop.

This might also be related to #44 because it seems like there the same thing is done with proxies and sessions.

internpoon commented 1 year ago

@bogdanfinn Initially i tried to create separate client for each go routine, it crashes as well, so i thought maybe too many instances of client could be the cause hence i changed to this one, still no luck.

Thanks for pointing out on the defer statement, should prolly wrap it inside an anonymous function.

bogdanfinn commented 1 year ago

@internpoon then lets cleanup your example function a bit to make it clearer what happens when and add some comments for you to understand:


func main() {
    logger := tls_client.NewLogger()

    options := []tls_client.HttpClientOption{
        tls_client.WithTimeoutSeconds(10),
        tls_client.WithClientProfile(tls_client.Chrome_112),
        tls_client.WithInsecureSkipVerify(),
        tls_client.WithNotFollowRedirects(),
    }

    proxies := getProxiesFromSomethere() 

    client, _ := tls_client.NewHttpClient(logger, options...)

    var wg sync.WaitGroup
    wg.Add(100)
    for i := 0; i < 100; i++ {
        go func(j int) {

            for {
                defer wg.Done()
                // rand.Seed(time.Now().Unix()) since go 1.20 not needed anymore

                proxyUrl := parseProxy(proxies[rand.Intn(len(proxies))])

                if err := client.SetProxy(proxyUrl); err != nil {
                    fmt.Println(err)
                }

                req, err := http.NewRequest(http.MethodGet, "https://www.ipinfo.io/json", nil)
                if err != nil {
                    log.Println(err)

                    // we have to continue here, because in the error case req will be nil
                    continue
                }

                resp, err := client.Do(req)
                if err != nil {
                    log.Println(err)

                    // we have to continue here, because in the error case resp will be nil and the next access on the statusCode will fail
                    continue
                }

                if resp.StatusCode == 502 {
                    // we continue silently
                    continue
                }

                d, err := io.ReadAll(resp.Body)
                if err != nil {
                    log.Println(err)

                    // we have to continue here, because in the error case resp will be nil and the next access on the statusCode will fail
                    continue
                }

                resp.Body.Close() // we close the body without defer, because we are inside a loop

                fmt.Println(fmt.Sprintf("%d: %s", resp.StatusCode, string(d)))
            }
        }(i)
    }

    wg.Wait()
}
bogdanfinn commented 1 year ago

In your example code when an error happens during the request then err is non nil and resp is nil. your are continue accessing resp in that case which is wrong.

internpoon commented 1 year ago

@bogdanfinn Thanks for the help. Appreciate it.