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: context cancellation can leave HTTP client with deadlocked HTTP/1.1 connections in Go1.22 [1.22 backport] #65759

Closed
gopherbot opened this issue Feb 16, 2024 · 3 comments
Closed

net/http: context cancellation can leave HTTP client with deadlocked HTTP/1.1 connections in Go1.22 [1.22 backport] #65759

gopherbot opened this issue Feb 16, 2024 · 3 comments
Assignees
@neild
Labels
CherryPickApproved Used during the release process for point releases
Milestone
Go1.22.1

Comments

@gopherbot
Copy link
Contributor

gopherbot commented Feb 16, 2024 •
edited by neild
Loading

@neild requested issue #65705 to be considered for backport to the next 1.22 minor release.

Thanks for the nice analysis (and the fix!).

@gopherbot please open backport issue for go1.22. This is significant regression with no workaround.

Edit: s/go1.21/go1.22/
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 16, 2024
@gopherbot gopherbot mentioned this issue Feb 16, 2024
Closed
@gopherbot gopherbot added this to the Go1.21.8 milestone Feb 16, 2024
@tibbes
Copy link
Contributor

tibbes commented Feb 20, 2024

I think this backport request was accidentally opened against the wrong release. The buggy code wasn't present in 1.21. This should be a backport request for the next 1.22 minor release.

Is it possible to switch the milestone on this issue?

@neild neild modified the milestones: Go1.21.8, Go1.22.1 Feb 21, 2024
@neild neild self-assigned this Feb 21, 2024
@neild neild changed the title net/http: context cancellation can leave HTTP client with deadlocked HTTP/1.1 connections in Go1.22 [1.21 backport] net/http: context cancellation can leave HTTP client with deadlocked HTTP/1.1 connections in Go1.22 [1.22 backport] Feb 21, 2024
@prattmic prattmic added the CherryPickApproved Used during the release process for point releases label Feb 21, 2024
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Feb 21, 2024
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/566536 mentions this issue: [release-branch.go1.22] net/http: add missing call to decConnsPerHost

gopherbot pushed a commit that referenced this issue Feb 28, 2024
@tibbes @cagedmantis
[release-branch.go1.22] net/http: add missing call to decConnsPerHost
16830ab 
A recent change to Transport.dialConnFor introduced an early return that
skipped dialing. This path did not call decConnsPerHost, which can cause
subsequent HTTP calls to hang if Transport.MaxConnsPerHost is set.

For #65705
Fixes #65759

Change-Id: I157591114b02a3a66488d3ead7f1e6dbd374a41c
Reviewed-on: https://go-review.googlesource.com/c/go/+/564036
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
(cherry picked from commit 098a87f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/566536
Reviewed-by: Carlos Amedee <carlos@golang.org>
@gopherbot
Copy link
Contributor Author

Closed by merging 16830ab to release-branch.go1.22.

bradfitz pushed a commit to tailscale/go that referenced this issue Mar 5, 2024
@tibbes @bradfitz
[release-branch.go1.22] net/http: add missing call to decConnsPerHost
729e510 
A recent change to Transport.dialConnFor introduced an early return that
skipped dialing. This path did not call decConnsPerHost, which can cause
subsequent HTTP calls to hang if Transport.MaxConnsPerHost is set.

For golang#65705
Fixes golang#65759

Change-Id: I157591114b02a3a66488d3ead7f1e6dbd374a41c
Reviewed-on: https://go-review.googlesource.com/c/go/+/564036
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
(cherry picked from commit 098a87f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/566536
Reviewed-by: Carlos Amedee <carlos@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 6, 2024
@tibbes @romaindoumenc
[release-branch.go1.22] net/http: add missing call to decConnsPerHost
3bd92e7 
A recent change to Transport.dialConnFor introduced an early return that
skipped dialing. This path did not call decConnsPerHost, which can cause
subsequent HTTP calls to hang if Transport.MaxConnsPerHost is set.

For golang#65705
Fixes golang#65759

Change-Id: I157591114b02a3a66488d3ead7f1e6dbd374a41c
Reviewed-on: https://go-review.googlesource.com/c/go/+/564036
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
(cherry picked from commit 098a87f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/566536
Reviewed-by: Carlos Amedee <carlos@golang.org>
chrisd8088 added a commit to chrisd8088/build-dockers that referenced this issue Mar 8, 2024
@chrisd8088
update to Go 1.22.1
6d70fc8 
Now that the Go 1.22.1 has been released with a backported fix
(golang/go@16830ab, per golang/go#65759)
for the problem reported in golang/go#65705, we can upgrade to Go 1.22
to align with our CI and release workflows in the main Git LFS repository.
@chrisd8088 chrisd8088 mentioned this issue Mar 8, 2024
Merged
@gabyhelp gabyhelp mentioned this issue Jun 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@neild neild

Labels
CherryPickApproved Used during the release process for point releases
Projects
None yet
Milestone
Go1.22.1
Development

No branches or pull requests
4 participants
@neild @prattmic @tibbes @gopherbot