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

net/http/httptrace: panic on GotConn [1.13 backport] #34285

Closed
gopherbot opened this issue Sep 13, 2019 · 10 comments
Closed

net/http/httptrace: panic on GotConn [1.13 backport] #34285

gopherbot opened this issue Sep 13, 2019 · 10 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.13.3

Comments

@gopherbot
Copy link
Contributor

@cuonglm requested issue #34282 to be considered for backport to the next 1.13 minor release.

@gopherbot please consider this for backport 1.13
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 13, 2019
@gopherbot gopherbot mentioned this issue Sep 13, 2019
Closed
@gopherbot gopherbot added this to the Go1.13.1 milestone Sep 13, 2019
@toothrot
Copy link
Contributor

@cuonglm Could you please provide a short rationale about why the backport might be needed to 1.13? See https://golang.org/wiki/MinorReleases, which describes the process.
@cuonglm
Copy link
Member

cuonglm commented Sep 17, 2019

@toothrot This is a regression from 1.12, and bug introduce in 1.13. It should be backport to 1.13.1, otherwise, user code which built with go1.13 will be broken, example see grafana/k6#1153
@cuonglm
Copy link
Member

cuonglm commented Sep 18, 2019

@mvdan @bradfitz Should this issue be backport?
@bradfitz
Copy link
Contributor

SGTM.
@bradfitz bradfitz added the CherryPickApproved Used during the release process for point releases label Sep 18, 2019
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Sep 18, 2019
@cuonglm
Copy link
Member

cuonglm commented Sep 19, 2019

@tmthrgd would you mind creating CL with the cherry pick.
@tmthrgd
Copy link
Contributor

tmthrgd commented Sep 20, 2019

@cuonglm I'm not sure how to do that, sorry. If someone else wants to create the cherry pick CL that would be appreciated.
@ALTree
Copy link
Member

ALTree commented Sep 20, 2019

@tmthrgd See: https://github.com/golang/go/wiki/MinorReleases#making-cherry-pick-cls
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/196579 mentions this issue: [release-branch.go1.13] net/http: fix HTTP/2 idle pool tracing
@tmthrgd
Copy link
Contributor

tmthrgd commented Sep 20, 2019

Thanks @ALTree, that was simple enough to follow.
@gopherbot
Copy link
Contributor Author

Closed by merging a055bb9 to release-branch.go1.13.
gopherbot pushed a commit that referenced this issue Sep 25, 2019
@tmthrgd @bradfitz
[release-branch.go1.13] net/http: fix HTTP/2 idle pool tracing
a055bb9 
CL 140357 caused HTTP/2 connections to be put in the idle pool, but
failed to properly guard the trace.GotConn call in getConn. dialConn
returns a minimal persistConn with conn == nil for HTTP/2 connections.
This persistConn was then returned from queueForIdleConn and caused the
httptrace.GotConnInfo passed into GotConn to have a nil Conn field.

HTTP/2 connections call GotConn themselves so leave it for HTTP/2 to call
GotConn as is done directly below.

Fixes #34285

Change-Id: If54bfaf6edb14f5391463f908efbef5bb8a5d78e
GitHub-Last-Rev: 2b7d66a
GitHub-Pull-Request: #34283
Reviewed-on: https://go-review.googlesource.com/c/go/+/195237
Reviewed-by: Michael Fraenkel <michael.fraenkel@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 582d519)
Reviewed-on: https://go-review.googlesource.com/c/go/+/196579
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
@bcmills bcmills modified the milestones: Go1.13.1, Go1.13.2 Sep 25, 2019
@cuonglm cuonglm mentioned this issue Sep 26, 2019
Merged
@katiehockman katiehockman modified the milestones: Go1.13.2, Go1.13.3 Oct 17, 2019
@golang golang locked and limited conversation to collaborators Oct 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
8 participants
@bradfitz @toothrot @tmthrgd @ALTree @cuonglm @bcmills @gopherbot @katiehockman