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

x/net/http2: singleUse ClientConn with AllowHTTP transport is always unusable #61863

Open
ctrlaltdel121 opened this issue Aug 8, 2023 · 1 comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ctrlaltdel121
Copy link

What version of Go are you using (go version)?

$ go version 
go version go1.20.2 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="arm64"
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPROXY="direct"
GOROOT="/opt/homebrew/Cellar/go/1.20.2/libexec"
GOSUMDB="sum.golang.org"
GOTOOLDIR="/opt/homebrew/Cellar/go/1.20.2/libexec/pkg/tool/darwin_arm64"
GOVERSION="go1.20.2"
GCCGO="gccgo"
AR="ar"
CC="cc"
CXX="c++"
CGO_ENABLED="1"
CGO_CFLAGS="-O2 -g"
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/g4/47vwdw1s65929qw7smyq1vlswhhzgj/T/go-build4150723989=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Create an HTTP2 transport with AllowHTTP = true, and try to send a request using it with req.Close = true. This will hang until the internal retry mechanism in the HTTP2 roundtrip ends after exponential backoff.

https://go.dev/play/p/KhmQcQB0PPD

If you remove req.Close = true, in my contrived example you get unexpected EOF, but the point is no timeout, because its not retrying aimlessly.

What did you expect to see?

I would expect the transport to understand that AllowHTTP=true setting nextStreamID to 3 in the newClientConn() function does not mean it should be considered "not usable" before it ever makes a request.

What did you see instead?

2009/11/10 23:00:00 Starting server...
2009/11/10 23:00:00 Sending request...
<time passes because of retry backoff>
http2: client conn not usable

The issue is because of this line that sets nextStreamID to 3 before a request is ever made, and then this line that interprets a nextStreamID higher than 1 as having been "used".

I'm not sure my use case for this is exactly "up to standard" but it seems like the behavior is unintentional - the connection should be usable once even if AllowHTTP was set on its creation, and at least it shouldn't retry exponentially when there's no way it could get a different outcome in the next iteration.

Thanks for reading!

@gopherbot gopherbot added this to the Unreleased milestone Aug 8, 2023
@mknyszek mknyszek changed the title x/net: http2 singleUse ClientConn with AllowHTTP transport is always unusable x/net/http2: singleUse ClientConn with AllowHTTP transport is always unusable Aug 8, 2023
@mknyszek
Copy link
Contributor

mknyszek commented Aug 8, 2023

CC @neild

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants