net/http: fix server/transport data race when sharing the request body

Introduced in https://go-review.googlesource.com/12865 (git rev c2db5f4c).

This fix doesn't add any new lock acquistions: it just moves the
existing one taken by the unreadDataSize method and moves it out
wider.

It became flaky at rev c2db5f4c, but now reliably passes again:
$ go test -v -race -run=TestTransportAndServerSharedBodyRace -count=100 net/http

Fixes #11985

Change-Id: I6956d62839fd7c37e2f7441b1d425793f4a0db30
Reviewed-on: https://go-review.googlesource.com/12909
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/net/http/server.go b/src/net/http/server.go
index 905a8b9ad8dac76901186b1eff1b1b048c386ea0..1b292ea2de39e248becfafa8cf0c41d4aa3c6985 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -880,17 +880,19 @@ func (cw *chunkWriter) writeHeader(p []byte) {
                                discard = true
                        }
                case *body:
+                       bdy.mu.Lock()
                        switch {
                        case bdy.closed:
                                if !bdy.sawEOF {
                                        // Body was closed in handler with non-EOF error.
                                        w.closeAfterReply = true
                                }
-                       case bdy.unreadDataSize() >= maxPostHandlerReadBytes:
+                       case bdy.unreadDataSizeLocked() >= maxPostHandlerReadBytes:
                                tooBig = true
                        default:
                                discard = true
                        }
+                       bdy.mu.Unlock()
                default:
                        discard = true
                }

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index d1762ebbd24cf766f5f94d3451834ffb137b22c8..c128a1d3cd439c1b92e9691055646c020ffb20c9 100644 (file)
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -737,11 +737,10 @@ func mergeSetHeader(dst *Header, src Header) {
        }
 }
 
-// unreadDataSize returns the number of bytes of unread input.
+// unreadDataSizeLocked returns the number of bytes of unread input.
 // It returns -1 if unknown.
-func (b *body) unreadDataSize() int64 {
-       b.mu.Lock()
-       defer b.mu.Unlock()
+// b.mu must be held.
+func (b *body) unreadDataSizeLocked() int64 {
        if lr, ok := b.src.(*io.LimitedReader); ok {
                return lr.N
        }