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

Webseed Client - Close body in same go routine as request #933

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

mh0lt
Copy link
Contributor

@mh0lt mh0lt commented Apr 21, 2024

This fix simplifies the go routine profile for http calls.

Instead of routine->channel-routine called one after the other (which is effectively synchronous the code executes in a single go routine.

This is to help elevate this go http related issue - which causes the torrent to panic:

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/[email protected]/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/[email protected]/webseed/client.go:97 +0x27
created by github.com/anacrolix/torrent/webseed.(*Client).NewRequest.func1.1 in goroutine 1030501
	github.com/anacrolix/[email protected]/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

@AskAlexSharov
Copy link
Collaborator

@anacrolix hi. Do you need my help with reviewing this PR?

@anacrolix
Copy link
Owner

@AskAlexSharov that would be appreciated!

Copy link
Collaborator

@AskAlexSharov AskAlexSharov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed readRequestPartResponses already run in own goroutine, don't need 2-nd.

@AskAlexSharov AskAlexSharov merged commit 7d0acb5 into anacrolix:master Apr 24, 2024
10 checks passed
AskAlexSharov added a commit that referenced this pull request Apr 24, 2024
Webseed Client - Close body in same go routine as request
@AskAlexSharov
Copy link
Collaborator

@anacrolix FYI: I created branch release/v1.54 to cherry-pick this PR there and maybe make v1.54.2 tag on that branch. Will take a look maybe need cherry-pick something else.
Diff: https://github.com/anacrolix/torrent/compare/v1.54.1...release/v1.54?expand=1

@anacrolix
Copy link
Owner

Ok sounds good.

AskAlexSharov added a commit that referenced this pull request Apr 25, 2024
Webseed Client - Close body in same go routine as request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants