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: transport RoundTrip fails when readLoop begins first #7822

Closed
fraenkel opened this issue Apr 19, 2014 · 11 comments
Closed

net/http: transport RoundTrip fails when readLoop begins first #7822

fraenkel opened this issue Apr 19, 2014 · 11 comments
Milestone

Comments

@fraenkel
Copy link
Contributor

go version devel +70af843fed44 Sat Apr 19 09:55:09 2014 +0200 darwin/amd64

I am using the ReverseProxy, and 90% of the time, I can get a Unsolicited response
received on idle HTTP channel starting with "H"; err=<nil>

The problem is pretty easy to see once you walk transport.RoundTrip().
There is no guarantee that numExpectedResponses is incremented in pconn.RoundTrip()
before pconn.ReadLoop() executes.

transport.dialConn() starts the pconn.readLoop() goroutine and returns which releases
transport.getConn() which then calls pconn.RoundTrip().
If the peek() in readLoop is slow, you won't ever see this problem, but I am not so
lucky.

Looks like the easiest/safest fix is to either move numExpectedResponses++ or just set
it to 1.
@bradfitz
Copy link
Contributor

Comment 1:

Please provide a self-contained example code to reproduce.

Status changed to WaitingForReply.

@fraenkel
Copy link
Contributor Author

Comment 3:

This tends to fail quite often on OSX.

Attachments:

  1. main.go (1638 bytes)

@davecheney
Copy link
Contributor

Comment 4:

There is virtually no error checking in your sample. Could you please add correct error
checking and verify that the problem remains.

@fraenkel
Copy link
Contributor Author

Comment 5:

The error checking won't matter.  I stripped most of it because it didn't affect what
was being reported from the Transport.
$ ./main
2014/04/19 21:43:53 Unsolicited response received on idle HTTP channel starting with
"H"; err=<nil>
2014/04/19 21:43:53 http: proxy error: net/http: transport closed before response was
received
I can add error checking but it won't solve the race between the readLoop starting and
numExpectedResponses being incremented far later in roundTrip.

@fraenkel
Copy link
Contributor Author

Comment 6:

Here is additional error checking.
An interesting bit of information.  If I compile with go build -a main.go, my chances
are higher to hit the problem than if I just do a go build main.go.
I don't know why that matters but it does.

Attachments:

  1. main.go (2141 bytes)

@davecheney
Copy link
Contributor

Comment 7:

Do you have two installations of Go on your system ?
Have you set GOROOT by accident ?
Can you include the output of go build -a main.go

@fraenkel
Copy link
Contributor Author

Comment 8:

Yes, I have 1.2.1 and tip.
Ok, the problem was some how I compiled in my fix into my tip and when I undid it, I
didn't recompile which is why I go the two behaviors.  Now that I have the original tip
compiled, the problem now appears regardless of how I build.

Attachments:

  1. main (5681020 bytes)

@davecheney
Copy link
Contributor

Comment 9:

Confirmed. Thank you.

Labels changed: added release-go1.3, repo-main.

Status changed to Accepted.

@bradfitz
Copy link
Contributor

Comment 10:

What's the bug here? The custom HTTP server in comment #6's main.go startApp actually
replies before it receives a request.
The numExpectedReplies++ in transport.go is already very early: it's before we even ask
the writeLoop to write the request.
If the server replies before we even try to write a request, the server really is
speaking out of turn. Looks working as intended to me.
What am I missing?

Labels changed: added release-go1.3maybe, removed release-go1.3.

Status changed to WaitingForReply.

@fraenkel
Copy link
Contributor Author

Comment 11:

You are correct.  The HTTP spec disallows early responses.  As long as you read
something, a response can be sent back.
This can be closed.

@ianlancetaylor
Copy link
Contributor

Comment 12:

Status changed to WorkingAsIntended.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants