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

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
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@g7r
Copy link
Contributor

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
Copy link
Contributor

bradfitz commented Apr 9, 2018

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

Could you take a look?

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. 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
Copy link
Contributor

meirf commented Apr 16, 2018 •
edited

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

@gopherbot
Copy link

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
@meirf @bradfitz
http2: terminate await request cancel goroutine on conn close
5f9ae10 
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
Copy link

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

@bradfitz bradfitz reopened this May 7, 2018
@gopherbot
Copy link

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

@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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
4 participants
@bradfitz @g7r @meirf @gopherbot