Commit 427a444f authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

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
parent a599b489
...@@ -719,7 +719,6 @@ type persistConn struct { ...@@ -719,7 +719,6 @@ type persistConn struct {
cacheKey connectMethodKey cacheKey connectMethodKey
conn net.Conn conn net.Conn
tlsState *tls.ConnectionState tlsState *tls.ConnectionState
closed bool // whether conn has been closed
br *bufio.Reader // from conn br *bufio.Reader // from conn
sawEOF bool // whether we've seen EOF from conn; owned by readLoop sawEOF bool // whether we've seen EOF from conn; owned by readLoop
bw *bufio.Writer // to conn bw *bufio.Writer // to conn
...@@ -733,8 +732,9 @@ type persistConn struct { ...@@ -733,8 +732,9 @@ type persistConn struct {
// whether or not a connection can be reused. Issue 7569. // whether or not a connection can be reused. Issue 7569.
writeErrCh chan error writeErrCh chan error
lk sync.Mutex // guards following 3 fields lk sync.Mutex // guards following fields
numExpectedResponses int 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. broken bool // an error has happened on this connection; marked broken so it's not reused.
// mutateHeaderFunc is an optional func to modify extra // mutateHeaderFunc is an optional func to modify extra
// headers on each outbound request before it's written. (the // headers on each outbound request before it's written. (the
...@@ -774,12 +774,14 @@ func (pc *persistConn) readLoop() { ...@@ -774,12 +774,14 @@ func (pc *persistConn) readLoop() {
pc.lk.Lock() pc.lk.Lock()
if pc.numExpectedResponses == 0 { if pc.numExpectedResponses == 0 {
if !pc.closed {
pc.closeLocked() pc.closeLocked()
pc.lk.Unlock()
if len(pb) > 0 { if len(pb) > 0 {
log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v", log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
string(pb), err) string(pb), err)
} }
}
pc.lk.Unlock()
return return
} }
pc.lk.Unlock() pc.lk.Unlock()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment