Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Downloader: http panic connCount underflow #9964

Closed
AskAlexSharov opened this issue Apr 17, 2024 · 2 comments
Closed

Downloader: http panic connCount underflow #9964

AskAlexSharov opened this issue Apr 17, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@AskAlexSharov
Copy link
Collaborator

AskAlexSharov commented Apr 17, 2024

(I thought that after #9962 easier to reproduce next panics, but seems not).

panic: net/http: internal error: connCount underflow

goroutine 1030540 [running]:
net/http.(*Transport).decConnsPerHost(0xc0008dc9a0, {{0x0, 0x0}, {0xc00afd8980, 0x5}, {0xc0274e0360, 0x2f}, 0x0})
	net/http/transport.go:1511 +0x405
net/http.(*Transport).roundTrip(0xc0008dc9a0, 0xc0039eb9e0)
	net/http/transport.go:618 +0x876
net/http.(*Transport).RoundTrip(...)
	net/http/roundtrip.go:17
github.com/ledgerwatch/erigon-lib/downloader.(*requestHandler).RoundTrip(0xc0008dc9a0, 0xc0039eb9e0)
	github.com/ledgerwatch/erigon-lib@v1.0.0/downloader/downloader.go:154 +0x11d
net/http.send(0xc0039eb9e0, {0x18533c0, 0xc0008dc9a0}, {0xc04c4bf501?, 0xc04c4bf788?, 0x0?})
	net/http/client.go:259 +0x5e4
net/http.(*Client).send(0xc000b927b0, 0xc0039eb9e0, {0x523de5?, 0xc0420d0b60?, 0x0?})
	net/http/client.go:180 +0x98
net/http.(*Client).do(0xc000b927b0, 0xc0039eb9e0)
	net/http/client.go:724 +0x8dc
net/http.(*Client).Do(...)
	net/http/client.go:590
github.com/anacrolix/torrent/webseed.(*Client).NewRequest.func1.1.1()
	github.com/anacrolix/torrent@v1.54.1/webseed/client.go:97 +0x27
created by github.com/anacrolix/torrent/webseed.(*Client).NewRequest.func1.1 in goroutine 1030501
	github.com/anacrolix/torrent@v1.54.1/webseed/client.go:96 +0x90
exit status 2

and

panic: net/http: internal error: connCount underflow

goroutine 63612 [running]:
net/http.(*Transport).decConnsPerHost(0xc000f894a0, {{0x0, 0x0}, {0xc02e36ef80, 0x5}, {0xc02b114ed0, 0x2f}, 0x0})
	net/http/transport.go:1511 +0x405
net/http.(*persistConn).closeLocked(0xc032db2900, {0x1852e20, 0x21bae10})
	net/http/transport.go:2764 +0xe5
net/http.(*persistConn).close(0xc000f894a0?, {0x1852e20?, 0x21bae10?})
	net/http/transport.go:2754 +0xa8
net/http.(*Transport).putOrCloseIdleConn(0xc000f894a0?, 0xc032db2900)
	net/http/transport.go:913 +0x2d
net/http.(*Transport).dialConnFor(0xc000f894a0, 0xc015572fd0)
	net/http/transport.go:1491 +0x11d
created by net/http.(*Transport).queueForDial in goroutine 57934
	net/http/transport.go:1461 +0x1cd
exit status 2

maybe it's because of some resp.Body not closed
maybe it's because another panic happened (but we don't see it) - maybe inside new RoundTrip func

@AskAlexSharov AskAlexSharov changed the title http panic after Downloader: http panic connCount underflow Apr 17, 2024
@mh0lt
Copy link
Contributor

mh0lt commented Apr 17, 2024

I think that the underlying issue is in:

https://github.com/anacrolix/torrent/blob/master/webseed/client.go

func readRequestPartResponses(ctx context.Context, parts []requestPart) (_ []byte, err error) {
	var buf bytes.Buffer
	for _, part := range parts {
		part.start()  <--This calls http.Client Do in a seperate go routine
		err = recvPartResult(ctx, &buf, part) <--This waits on a channel in this go routine and calls resp.Body.Close()
		if err != nil {
			err = fmt.Errorf("reading %q at %q: %w", part.req.URL, part.req.Header.Get("Range"), err)
			break
		}
	}
	return buf.Bytes(), err
}

The code is here:

part.start = func() {
			go func() {
				resp, err := ws.HttpClient.Do(req)
				part.result <- requestPartResult{
					resp: resp,
					err:  err,
				}
			}()
}

and here:

func recvPartResult(ctx context.Context, buf io.Writer, part requestPart) error {
	result := <-part.result
	// Make sure there's no further results coming, it should be a one-shot channel.
	close(part.result)
	if result.err != nil {
		return result.err
	}
	defer result.resp.Body.Close()
	var body io.Reader = result.resp.Body

I dont think the inner go routine is required as the calls are already asynchronous so start can be just:

  func() (resp *http.Response, err  error) {
     return ws.HttpClient.Do(req)
  }()

I think it will remove the potential race condition

@yperbasis yperbasis added the bug Something isn't working label Apr 24, 2024
@AskAlexSharov
Copy link
Collaborator Author

fixed by anacrolix/torrent#933

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants