[release-branch.go1.15] net/http: ignore connection closes once done with the connection

Once the connection is put back into the idle pool, the request should
not take any action if the connection is closed.

For #42935.
Updates #41600.

Change-Id: I5e4ddcdc03cd44f5197ecfbe324638604961de84
Reviewed-on: https://go-review.googlesource.com/c/go/+/257818
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Damien Neil <dneil@google.com>
(cherry picked from commit 212d385a2f723a8dd5e7d2e83efb478ddd139349)
Reviewed-on: https://go-review.googlesource.com/c/go/+/297909
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index d37b52b13d06006619d341692680bf13cc1a4d52..6fb2ea5663f6a7bec809557d21b94320f36c6e63 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -2560,6 +2560,7 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err
        var respHeaderTimer <-chan time.Time
        cancelChan := req.Request.Cancel
        ctxDoneChan := req.Context().Done()
+       pcClosed := pc.closech
        for {
                testHookWaitResLoop()
                select {
@@ -2579,11 +2580,15 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err
                                defer timer.Stop() // prevent leaks
                                respHeaderTimer = timer.C
                        }
-               case <-pc.closech:
-                       if debugRoundTrip {
-                               req.logf("closech recv: %T %#v", pc.closed, pc.closed)
+               case <-pcClosed:
+                       pcClosed = nil
+                       // check if we are still using the connection
+                       if pc.t.replaceReqCanceler(req.cancelKey, nil) {
+                               if debugRoundTrip {
+                                       req.logf("closech recv: %T %#v", pc.closed, pc.closed)
+                               }
+                               return nil, pc.mapRoundTripError(req, startBytesWritten, pc.closed)
                        }
-                       return nil, pc.mapRoundTripError(req, startBytesWritten, pc.closed)
                case <-respHeaderTimer:
                        if debugRoundTrip {
                                req.logf("timeout waiting for response headers.")

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 0a47687d9afbacac2ac7411c2e91e41b8b0915a9..3c7b9eb4de80a48fb3257c1e211cf0cf2bc92eb4 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6289,3 +6289,54 @@ func TestTransportRejectsSignInContentLength(t *testing.T) {
                t.Fatalf("Error mismatch\nGot: %q\nWanted substring: %q", got, want)
        }
 }
+
+// Issue 41600
+// Test that a new request which uses the connection of an active request
+// cannot cause it to be canceled as well.
+func TestCancelRequestWhenSharingConnection(t *testing.T) {
+       if testing.Short() {
+               t.Skip("skipping in short mode")
+       }
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, req *Request) {
+               w.Header().Add("Content-Length", "0")
+       }))
+       defer ts.Close()
+
+       client := ts.Client()
+       transport := client.Transport.(*Transport)
+       transport.MaxIdleConns = 1
+       transport.MaxConnsPerHost = 1
+
+       var wg sync.WaitGroup
+
+       ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+
+       for i := 0; i < 10; i++ {
+               wg.Add(1)
+               go func() {
+                       defer wg.Done()
+                       for ctx.Err() == nil {
+                               reqctx, reqcancel := context.WithCancel(ctx)
+                               go reqcancel()
+                               req, _ := NewRequestWithContext(reqctx, "GET", ts.URL, nil)
+                               res, err := client.Do(req)
+                               if err == nil {
+                                       res.Body.Close()
+                               }
+                       }
+               }()
+       }
+
+       for ctx.Err() == nil {
+               req, _ := NewRequest("GET", ts.URL, nil)
+               if res, err := client.Do(req); err != nil {
+                       t.Errorf("unexpected: %p %v", req, err)
+                       break
+               } else {
+                       res.Body.Close()
+               }
+       }
+
+       cancel()
+       wg.Wait()
+}