net/http/httputil: request may be killed by mistake by former cancel request in ReverseProxy.ServeHTTP

[xuzhaokui]

Hi:

1. What version of Go are you using (go version)?

   go version go1.6.2 darwin/amd64

2. What operating system and processor architecture are you using (go env)?

   GOARCH="amd64"
   GOOS="darwin"

3. What did you do?

   1. serving proxy requests by httputil.ReverseProxy
   2. about 10% of request are canceled by clients

4. What did you expect to see?

   no requests should occur any unexpected error.

5. What did you see instead?

   some normal(non-canceled) requests are affected by their former requests which are canceled by clients.

Finally I find this:

There is a goroutine here(in ServeHTTP): goroutine

this goroutine is used for receiving clientGone or reqDone signal, however, we assume this scene:

1. ServeHTTP start handling Request A(using underlying net.Conn C)
2. Request A was canceled by its client (sending sig to clientGone)
3. RoundTrip return error(may be use of closed network connection in go1.6- or other error in go1.6+)
4. ServeHTTP start to return and defer close(reqDone) is executed(close is non-block, so ServeHTTP may be return before the goroutine A receive close signal, but goroutine A is still alive)
5. net/http put net.Conn C to IdleConn
6. Request B arrived and net/http pick the same underlying net.Conn C
7. ServeHTTP start handling Request B (using the same underlying net.Conn C)
8. goroutine A got signal clientGone first, and then do CancelRequest which will close net.Conn C
9. Request B's RoundTrip return error cause its underlying conn is closed

[ianlancetaylor]

CC @adg

[adg]

Are you able to write a test that demonstrates the issue? I have a couple of ideas what might be wrong, but without a test I can't be sure.

[danp]

If it helps at all I did spend some time looking at this and trying to reproduce with various means, such as adding delay between the clientGone close being noticed and CancelRequest being called. And I tried with both 1.6.2 and tip post-1.7beta2. Wasn't able to come up with anything but happy to test further if there are any hunches.

Any more info on requests (specifically ones like "Request A") would be useful, such as what HTTP method they used, whether they had a body, etc.

[bradfitz]

I don't think step 8 in your scenario (requestCanceler.CancelRequest(outreq)) should close the underlying TCP connection. If step 3 happens (the RoundTrip failed), the cancellation code to close the TCP connection should've already been unregistered.

In any case, more details as requested by @adg and @dpiddy would be nice. How can we reproduce this? What do the requests look like? How are the 10% of requests being canceled? I'd like to help if I can see it fail.

Thanks.

For now, kicking to Go 1.8, unless you can get back to us soon.

[bradfitz]

Closing due to timeout. Let us know and reopen if you have more details.