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: Transport leaks goroutines (awaitRequestCancel) #24776

Closed
g7r opened this issue Apr 9, 2018 · 5 comments
Closed

x/net/http2: Transport leaks goroutines (awaitRequestCancel) #24776

g7r opened this issue Apr 9, 2018 · 5 comments

Comments

@g7r
Copy link
Contributor

@g7r g7r commented Apr 9, 2018

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

go version go1.10 darwin/amd64
go version go1.10.1 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/sz/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/sz/src/gohome"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/9h/x_80x8ln3_g038_zj_t9tszw0000gn/T/go-build456657465=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/sz/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/sz/go"
GORACE=""
GOROOT="/home/sz/goroot/1.10.1/go"
GOTMPDIR=""
GOTOOLDIR="/home/sz/goroot/1.10.1/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build220690427=/tmp/go-build -gno-record-gcc-switches"

What did you do?

HTTP2 client leaks goroutines when the following conditions are met:

  1. http.Client has Timeout set
  2. TLS connections are forcibly dropped
  3. MaxConcurrentStreams is reasonably low

Test program that reproduces the issue: https://gist.github.com/g7r/3044f20a47e5d41630d9a2cdc5c24918

What did you expect to see?

It is expected to see steady number of goroutines.

What did you see instead?

Number of goroutines grows infinitely.
Leaked goroutine stacktraces are:

#	0x1243f5f	net/http.http2awaitRequestCancel+0x13f				/usr/local/Cellar/go/1.10/libexec/src/net/http/h2_bundle.go:6780
#	0x127ca8b	net/http.(*http2ClientConn).awaitOpenSlotForRequest.func1+0x3b	/usr/local/Cellar/go/1.10/libexec/src/net/http/h2_bundle.go:7519
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Apr 9, 2018

@tombergan, that code was added in golang/net@1c05540 (https://go-review.googlesource.com/53250)

Could you take a look?

@bradfitz bradfitz added the NeedsFix label Apr 9, 2018
@bradfitz bradfitz added this to the Go1.11 milestone Apr 9, 2018
@bradfitz bradfitz changed the title HTTP/2 client leaks goroutines x/net/http2: Transport leaks goroutines (awaitRequestCancel) Apr 9, 2018
@meirf

This comment has been minimized.

Copy link
Contributor

@meirf meirf commented Apr 16, 2018

(I think I found the bug. Now I'm working on a test. Should have CL in next couple days.)

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Apr 20, 2018

Change https://golang.org/cl/108415 mentions this issue: http2: terminate await request cancel goroutine on conn close

gopherbot pushed a commit to golang/net that referenced this issue Apr 20, 2018
If conn closes but the request cancel select is still blocked
we must close the connection wait channel.

Updates golang/go#24776 (needs bundle into std for fix)

Change-Id: I7e3ffdf2dd9b127727e24fe262b2f4c5d96fc184
Reviewed-on: https://go-review.googlesource.com/108415
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 5, 2018

Change https://golang.org/cl/111655 mentions this issue: net/http: update bundled http2

@gopherbot gopherbot closed this in 260ae19 May 6, 2018
@bradfitz bradfitz reopened this May 7, 2018
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 7, 2018

Change https://golang.org/cl/111876 mentions this issue: vendor, net/http: update x/net for httplex to httpguts merge

@gopherbot gopherbot closed this in 8e9386d May 7, 2018
@golang golang locked and limited conversation to collaborators May 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.