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: close of Body closes keep-alive connection #5645

Closed
anacrolix opened this issue Jun 6, 2013 · 7 comments

Comments

Projects
None yet
4 participants
@anacrolix
Copy link
Contributor

commented Jun 6, 2013

http://play.golang.org/p/Sbqj2kmukv

What is the expected output?
I'm not sure, but I was under the impression the body would be discarded. (the program
won't print "closed", but it does)

What do you see instead?
The connection is closed if the body has non-zero length.

go version devel +ca145611fa96 Wed Jun 05 10:39:06 2013 -0400 linux/amd64
@bradfitz

This comment has been minimized.

Copy link
Member

commented Jun 10, 2013

Comment 2:

This is on purpose.
The way it was before was surprising and made things impossible. The way it is now means
it does what you say, but you can still wrap the ReadCloser with one that reads-to-EOF
on Close, if so desired.

Status changed to WorkingAsIntended.

@anacrolix

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2013

Comment 3:

Thanks for the clarification. The change in behaviour was surprising and caused
significant performance degradation in some apps I maintain.
Can you tell me when the change in behaviour occurred, and ensure the documentation is
clarified? It relevant part I find currently reads: "The client must close the response
body when finished with it" which is insufficient detail.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Jun 10, 2013

Comment 4:

I'm not sure I want to clarify (read: guarantee) the behavior either way. I reserve the
right to change the heuristic in the future. (e.g. maybe read-to-EOF if I know EOF is
coming "soon", for some definition of "soon", or as a function of whether SPDY is
enabled, or whether TCP Fast Open is enabled for that host, etc, etc)
But probably I just won't be changing it now.
@anacrolix

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2013

Comment 5:

Could you perhaps document that keep alive isn't guaranteed unless the response body is
discarded?
@bgentry

This comment has been minimized.

Copy link
Contributor

commented Nov 18, 2014

Comment 6:

I just discovered this issue and was surprised that I've been reading JSON from HTTP
responses incorrectly this whole time. I was even more surprised that nowhere in the
docs does it say you need to read the entire Body if you want the conn to be reused,
though it does say that you need to call Close().
Here's an example that shows how it's so easy to do this wrong, not knowing that some
extra whitespace after the JSON object could cause conns not to be reused:
http://play.golang.org/p/QMc9Dt0btS
If you don't think you should change the behavior, I really think it should at least be
documented.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Nov 18, 2014

Comment 7:

Okay, I'll document something for Go 1.5.
Note that in HTTP/2 this whole issue is moot.

Labels changed: added release-go1.5, repo-main, documentation, removed priority-triage.

Owner changed to @bradfitz.

Status changed to Accepted.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 13, 2014

@bradfitz bradfitz added the started label Dec 13, 2014

@bradfitz bradfitz closed this in 8476d4c Dec 13, 2014

@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014

@bradfitz bradfitz removed the release-go1.5 label Dec 16, 2014

@golang golang locked and limited conversation to collaborators Jun 24, 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.