net/http: fix another data race when sharing Request.Body

Fix another issue (similar to Issue 6995) where there was a
data race when sharing a server handler's Request.Body with
another goroutine that out-lived the Handler's goroutine.

In some cases we were not closing the incoming Request.Body
(which would've required reading it until the end) if we
thought it we thought we were going to be forcibly closing the
underlying net.Conn later anyway. But that optimization
largely moved to the transfer.go *body later, and locking was
added to *body which then detected read-after-close, so now
calling the (*body).Close always is both cheap and correct.

No new test because TestTransportAndServerSharedBodyRace caught it,
albeit only sometimes. Running:

while ./http.test -test.cpu=8 -test.run=TestTransportAndServerSharedBodyRace; do true; done

... would reliably cause a race before, but not now.

Update #6995
Fixes #7092

R=golang-codereviews, khr
CC=golang-codereviews
https://golang.org/cl/51700043

diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go
index a56aa3df312a0f2743873c339a0ceebca8ab31c4..778089aa3efcf98e9dde6b49f186a9c057543828 100644 (file)
--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -997,11 +997,10 @@ func (w *response) finishRequest() {
        w.cw.close()
        w.conn.buf.Flush()
 
-       // Close the body, unless we're about to close the whole TCP connection
-       // anyway.
-       if !w.closeAfterReply {
-               w.req.Body.Close()
-       }
+       // Close the body (regardless of w.closeAfterReply) so we can
+       // re-use its bufio.Reader later safely.
+       w.req.Body.Close()
+
        if w.req.MultipartForm != nil {
                w.req.MultipartForm.RemoveAll()
        }