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.

Sign up for GitHub

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

Jump to bottom

Allow connection close for custom streams #1603

Merged
merged 4 commits into from
Sep 2, 2023
Merged

Allow connection close for custom streams #1603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a40b30
feat: allow connection close for custom streams
becheran Aug 11, 2023
caa857b
fix: avoid req access since might already be released
becheran Aug 11, 2023
7ccc02a
fix: fix aloc test fails
becheran Aug 28, 2023
56c46ea
fix: race condition when clossing body stream
becheran Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2893,8 +2893,8 @@ func (t *transport) RoundTrip(hc *HostClient, req *Request, resp *Response) (ret

br := hc.acquireReader(conn)
err = resp.ReadLimitBody(br, hc.MaxResponseBodySize)
hc.releaseReader(br)
if err != nil {
hc.releaseReader(br)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erikdubbelboer this is another change that would be required to avoid data races if the connection can be closed from outside. The reader shall only be released directly if it is not a stream connection. Otherwise do it in the cleanup method called via the closer interface.

The reason is that if the reader is realeased earlier, it would then be available for the next request and might cause a data race.

hc.closeConn(cc)
// Don't retry in case of ErrBodyTooLarge since we will just get the same again.
needRetry := err != ErrBodyTooLarge
Expand All @@ -2905,17 +2905,20 @@ func (t *transport) RoundTrip(hc *HostClient, req *Request, resp *Response) (ret
if customStreamBody && resp.bodyStream != nil {
rbs := resp.bodyStream
resp.bodyStream = newCloseReader(rbs, func() error {
hc.releaseReader(br)
if r, ok := rbs.(*requestStream); ok {
releaseRequestStream(r)
}
if closeConn {
if closeConn || resp.ConnectionClose() {
hc.closeConn(cc)
} else {
hc.releaseConn(cc)
}
return nil
})
return false, nil
} else {
hc.releaseReader(br)
}

if closeConn {
Expand Down
2 changes: 1 addition & 1 deletion http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,8 +1106,8 @@ func (resp *Response) Reset() {
if responseBodyPoolSizeLimit >= 0 && resp.body != nil {
resp.ReleaseBody(responseBodyPoolSizeLimit)
}
resp.Header.Reset()
resp.resetSkipHeader()
resp.Header.Reset()
erikdubbelboer marked this conversation as resolved.
Show resolved Hide resolved
resp.SkipBody = false
resp.raddr = nil
resp.laddr = nil
Expand Down
Loading