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: flake in TestHijackAfterCloseNotifier? #13633

Closed
mwhudson opened this issue Dec 16, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@mwhudson
Copy link
Contributor

commented Dec 16, 2015

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Dec 16, 2015

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 16, 2015

I can reproduce at least on Linux/amd64.

Notes:

  • 0 to 3 in 2000 runs fails for me.
  • creating a dedicated Transport just for this test (e.g. with newClientServerTest) doesn't fix it.
  • disabling the CloseNotify calls doesn't fix it.
  • closing Response.Bodies doesn't fix it.
  • copying the bytes from Response.Body to ioutil.Discard (always zero, always nil io.Copy error) doesn't fix it.
  • no races detected
  • even a http.Handler of this single line still fails:
   w.Header().Set("X-Addr", r.RemoteAddr)

So this has nothing to do with either CloseNotifier, nor Hijack.

There are other tests which use this same pattern of doing two requests and verifying they report the same RemoteAddr as a proxy for determining whether the connection was re-used. One example is TestHandlerSetsBodyNil_h1 and _h2. It looks identical about I can't get it to flake.

I'm very confused.

@bradfitz bradfitz self-assigned this Dec 16, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 16, 2015

Ah, the difference between TestHijackAfterCloseNotifier and TestHandlerSetsBodyNil_h1 is that the latter writes non-zero response bytes. And indeed, that seems to be the cause of the flakiness: if there are zero response bytes and a Content-Length of 0, the Transport sometimes creates a new connection rather than re-using the one it just replied with.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 16, 2015

@bradfitz bradfitz closed this in 1fe3933 Dec 16, 2015

@mwhudson

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2015

Thanks for the quick fix :-)

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