Goroutine leak

[3mard]

I've noticed that running the speed test in a loop can cause a goroutine/memory leak

Consider the following code being run in a loop (sequentially )

	serverList, err := a.tst.FetchServers()
		if err != nil {
			fmt.Println(err)
			return
		}
		targets, _ := serverList.FindServer([]int{})
		if len(targets) == 0 {
			fmt.Println("No servers found")
			return
		}
		s := targets[0]
		s.PingTest(nil)
		s.DownloadTest()
		s.UploadTest()

		fmt.Printf("Latency: %s, Download: %f, Upload: %f\n", s.Latency, s.DLSpeed, s.ULSpeed)
		UploadSpeed.Set(s.ULSpeed)
		DownloadSpeed.Set(s.DLSpeed)
		Latency.Set(float64(s.Latency.Milliseconds()))
		s.Context.Reset() // reset counter
		a.tst.Reset()

Will result in an increasing goroutines count that seems to never decrease, eg:

1390 @ 0x4d398 0x5e5a8 0x2babfc 0x80894
#	0x2babfb	net/http.(*persistConn).readLoop+0xb7b	/usr/local/go/src/net/http/transport.go:2238

1390 @ 0x4d398 0x5e5a8 0x2bb99c 0x80894
#	0x2bb99b	net/http.(*persistConn).writeLoop+0x9b	/usr/local/go/src/net/http/transport.go:2421

Upon my investigation I found the following function that seems to me to be the source of the leak

func (s *Server) HTTPPing(
	ctx context.Context,
	echoTimes int,
	echoFreq time.Duration,
	callback func(latency time.Duration),
) (latencies []int64, err error) {
	u, err := url.Parse(s.URL)
	if err != nil || len(u.Host) == 0 {
		return nil, err
	}
	pingDst := fmt.Sprintf("%s/latency.txt", s.URL)
	dbg.Printf("Echo: %s\n", pingDst)
	failTimes := 0
	req, err := http.NewRequestWithContext(ctx, http.MethodGet, pingDst, nil)
	if err != nil {
		return nil, err
	}
	for i := 0; i < echoTimes; i++ {
		sTime := time.Now()
		_, err = s.Context.doer.Do(req)
		endTime := time.Since(sTime)
		if err != nil {
			failTimes++
			continue
		}
		latencies = append(latencies, endTime.Nanoseconds()/2)
		dbg.Printf("2RTT: %s\n", endTime)
		if callback != nil {
			callback(endTime / 2)
		}
		time.Sleep(echoFreq)
	}
	if failTimes == echoTimes {
		return nil, ErrConnectTimeout
	}
	return
}

The problem lays in _, err = s.Context.doer.Do(req), According to the docs

If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close. If the Body is not both read to EOF and closed, the Client's underlying RoundTripper (typically Transport) may not be able to re-use a persistent TCP connection to the server for a subsequent "keep-alive" request.

[r3inbowari]

@3mard, Thank you for your careful investigation!

[r3inbowari]

But that change made func TestFetchServerList(t *testing.T) fail
https://github.com/showwin/speedtest-go/actions/runs/7324003922/job/19947154035

[3mard]

Sorry I made a mistake #167 should fix it

However TestFetchServerByID is still failing with
server_test.go:167: no server available or found

[r3inbowari]

Thank you, it works.

[3mard]

Sure thing :) should we close this issue now ?