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

net/http: logical race in (*net/http.Transport).CancelRequest #10474

Closed
rhysh opened this issue Apr 16, 2015 · 2 comments

Comments

Projects
None yet
4 participants
@rhysh
Copy link
Contributor

commented Apr 16, 2015

With go version go1.4.1 linux/amd64, there's a logical race in (*net/http.Transport).CancelRequest between the readLoop goroutine at https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/transport.go#L935 and the goroutine that calls Close on the response Body at https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/transport.go#L908 .

The closure on line 908 can cause the net.Conn to be handed off to the connection pool before the canceler is cleared on line 935. A call to CancelRequest at this time will then close the connection, even though it's been returned to the pool. The next request to pull that net.Conn out of the pool will fail.

Contexts make this condition easy to trigger: if a monitor goroutine waits on ctx.Done before calling CancelRequest, it's easy to come up with code that would cause the Context to be canceled upon successful completion of the request (e.g. to clean up after context.WithTimeout).

@rhysh rhysh changed the title logical race in (*net/http.Transport).CancelRequest net/http: logical race in (*net/http.Transport).CancelRequest Apr 16, 2015

@DanielMorsing

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2015

Confirmed with this program https://play.golang.org/p/M1-NTl3odt

@DanielMorsing DanielMorsing self-assigned this Apr 19, 2015

@DanielMorsing DanielMorsing added this to the Go1.5 milestone Apr 19, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 20, 2015

This is a dup of #9496 but both have good information.
In any case, https://go-review.googlesource.com/8550 fixes it.

@bradfitz bradfitz closed this in b016eba Apr 20, 2015

@golang golang locked and limited conversation to collaborators Jun 25, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.