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

http2: revert more of upstream http2 change #1

Merged
merged 1 commit into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 7 additions & 24 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -1266,29 +1266,9 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
return res, nil
}

cancelRequest := func(cs *clientStream, markDoNotReuse bool, err error) error {
cancelRequest := func(cs *clientStream, err error) error {
cs.cc.mu.Lock()
cs.abortStreamLocked(err)
bodyClosed := cs.reqBodyClosed
if markDoNotReuse && cs.ID != 0 {
// This request may have failed because of a problem with the connection,
// or for some unrelated reason. (For example, the user might have canceled
// the request without waiting for a response.) Mark the connection as
// not reusable, since trying to reuse a dead connection is worse than
// unnecessarily creating a new one.
//
// If cs.ID is 0, then the request was never allocated a stream ID and
// whatever went wrong was unrelated to the connection. We might have
// timed out waiting for a stream slot when StrictMaxConcurrentStreams
// is set, for example, in which case retrying on a different connection
// will not help.
cs.cc.doNotReuse = true

if f := cs.cc.t.CountError; f != nil {
f("abort_set_do_not_reuse")
log.Printf("ts-http2: set do not reuse: %T, %v", err, err)
}
}
cs.cc.mu.Unlock()
// Wait for the request body to be closed.
//
Expand Down Expand Up @@ -1323,12 +1303,15 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
return handleResponseHeaders()
default:
waitDone()
return nil, cancelRequest(cs, true, cs.abortErr)
return nil, cs.abortErr
}
case <-ctx.Done():
return nil, cancelRequest(cs, false, ctx.Err())
err := ctx.Err()
cs.abortStream(err)
return nil, cancelRequest(cs, err)
case <-cs.reqCancel:
return nil, cancelRequest(cs, false, errRequestCanceled)
cs.abortStream(errRequestCanceled)
return nil, cancelRequest(cs, errRequestCanceled)
}
}
}
Expand Down
1 change: 1 addition & 0 deletions http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3857,6 +3857,7 @@ func TestTransportRetryAfterGOAWAY(t *testing.T) {
}

func TestTransportRetryAfterRefusedStream(t *testing.T) {
t.Skip("broken by 82780d60 and later reverts; see https://github.com/golang/go/issues/60818 and https://github.com/tailscale/corp/issues/12296")
clientDone := make(chan struct{})
client := func(tr *Transport) {
defer close(clientDone)
Expand Down