net/http: fix data race in client

Fixes #73522

Co-authored-by: Damien Neil <dneil@google.com>
Change-Id: I6fb408a0b03bc387f443e17e6f9d0bac32eff31e
Reviewed-on: https://go-review.googlesource.com/c/go/+/694815
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 07b3a9e1e72ba60122d83d48558d91fb464b47b7..b860eb95b043da5a14a869aaf9efab30bc074dff 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -722,7 +722,7 @@ func (t *Transport) roundTrip(req *Request) (_ *Response, err error) {
                        if e, ok := err.(transportReadFromServerError); ok {
                                err = e.err
                        }
-                       if b, ok := req.Body.(*readTrackingBody); ok && !b.didClose {
+                       if b, ok := req.Body.(*readTrackingBody); ok && !b.didClose.Load() {
                                // Issue 49621: Close the request body if pconn.roundTrip
                                // didn't do so already. This can happen if the pconn
                                // write loop exits without reading the write request.
@@ -752,8 +752,8 @@ var errCannotRewind = errors.New("net/http: cannot rewind body after connection
 
 type readTrackingBody struct {
        io.ReadCloser
-       didRead  bool
-       didClose bool
+       didRead  bool // not atomic.Bool because only one goroutine (the user's) should be accessing
+       didClose atomic.Bool
 }
 
 func (r *readTrackingBody) Read(data []byte) (int, error) {
@@ -762,7 +762,9 @@ func (r *readTrackingBody) Read(data []byte) (int, error) {
 }
 
 func (r *readTrackingBody) Close() error {
-       r.didClose = true
+       if !r.didClose.CompareAndSwap(false, true) {
+               return nil
+       }
        return r.ReadCloser.Close()
 }
 
@@ -784,10 +786,10 @@ func setupRewindBody(req *Request) *Request {
 // rewindBody takes care of closing req.Body when appropriate
 // (in all cases except when rewindBody returns req unmodified).
 func rewindBody(req *Request) (rewound *Request, err error) {
-       if req.Body == nil || req.Body == NoBody || (!req.Body.(*readTrackingBody).didRead && !req.Body.(*readTrackingBody).didClose) {
+       if req.Body == nil || req.Body == NoBody || (!req.Body.(*readTrackingBody).didRead && !req.Body.(*readTrackingBody).didClose.Load()) {
                return req, nil // nothing to rewind
        }
-       if !req.Body.(*readTrackingBody).didClose {
+       if !req.Body.(*readTrackingBody).didClose.Load() {
                req.closeBody()
        }
        if req.GetBody == nil {