net/http: quiet useless warning during shutdown

What was happening on Issue 7010 was handler intentionally took 30
milliseconds and the proxy's client timeout was 35 milliseconds. Then it
slammed the proxy with a bunch of requests.

Sometimes the server would be too slow to respond in its 5 millisecond
window and the client code would cancel the request, force-closing the
persistConn.  If this came at the right time, the server's reply was
already in flight, and one of the goroutines would report:

Unsolicited response received on idle HTTP channel starting with "H"; err=<nil>

... rightfully scaring the user.

But the error was already handled and returned to the user, and this
connection knows it's been shut down. So look at the closed flag after
acquiring the same mutex guarding another field we were checking, and
don't complain if it's a known shutdown.

Also move closed down below the mutex which guards it.

Fixes #7010

LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://golang.org/cl/86740044

diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go
index 1d776c26806195a0429116e48433a4e0366553a8..75d013eac3e9e9c1d95609a6e0b91584100f3c56 100644 (file)
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -719,7 +719,6 @@ type persistConn struct {
        cacheKey connectMethodKey
        conn     net.Conn
        tlsState *tls.ConnectionState
-       closed   bool                // whether conn has been closed
        br       *bufio.Reader       // from conn
        sawEOF   bool                // whether we've seen EOF from conn; owned by readLoop
        bw       *bufio.Writer       // to conn
@@ -733,8 +732,9 @@ type persistConn struct {
        // whether or not a connection can be reused. Issue 7569.
        writeErrCh chan error
 
-       lk                   sync.Mutex // guards following 3 fields
+       lk                   sync.Mutex // guards following fields
        numExpectedResponses int
+       closed               bool // whether conn has been closed
        broken               bool // an error has happened on this connection; marked broken so it's not reused.
        // mutateHeaderFunc is an optional func to modify extra
        // headers on each outbound request before it's written. (the
@@ -774,12 +774,14 @@ func (pc *persistConn) readLoop() {
 
                pc.lk.Lock()
                if pc.numExpectedResponses == 0 {
-                       pc.closeLocked()
-                       pc.lk.Unlock()
-                       if len(pb) > 0 {
-                               log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
-                                       string(pb), err)
+                       if !pc.closed {
+                               pc.closeLocked()
+                               if len(pb) > 0 {
+                                       log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
+                                               string(pb), err)
+                               }
                        }
+                       pc.lk.Unlock()
                        return
                }
                pc.lk.Unlock()