net/http2: wait until the request body has been written
When the clientConn is done with a request, if there is a request body,
we must wait until the body is written otherwise there can be a race
when attempting to rewind the body.
Fixes golang/go#31192
Fixes golang/go#41234
Change-Id: I77606cc19372eea8bbd8995102354f092b8042be
Reviewed-on: https://go-review.googlesource.com/c/net/+/253259
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
diff --git a/http2/transport.go b/http2/transport.go
index 974a3c2..8b129b7 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -1148,6 +1148,9 @@
// we can keep it.
bodyWriter.cancel()
cs.abortRequestBodyWrite(errStopReqBodyWrite)
+ if hasBody && !bodyWritten {
+ <-bodyWriter.resc
+ }
}
if re.err != nil {
cc.forgetStreamID(cs.ID)
@@ -1168,6 +1171,7 @@
} else {
bodyWriter.cancel()
cs.abortRequestBodyWrite(errStopReqBodyWriteAndCancel)
+ <-bodyWriter.resc
}
cc.forgetStreamID(cs.ID)
return nil, cs.getStartedWrite(), errTimeout
@@ -1177,6 +1181,7 @@
} else {
bodyWriter.cancel()
cs.abortRequestBodyWrite(errStopReqBodyWriteAndCancel)
+ <-bodyWriter.resc
}
cc.forgetStreamID(cs.ID)
return nil, cs.getStartedWrite(), ctx.Err()
@@ -1186,6 +1191,7 @@
} else {
bodyWriter.cancel()
cs.abortRequestBodyWrite(errStopReqBodyWriteAndCancel)
+ <-bodyWriter.resc
}
cc.forgetStreamID(cs.ID)
return nil, cs.getStartedWrite(), errRequestCanceled
@@ -1195,6 +1201,7 @@
// forgetStreamID.
return nil, cs.getStartedWrite(), cs.resetErr
case err := <-bodyWriter.resc:
+ bodyWritten = true
// Prefer the read loop's response, if available. Issue 16102.
select {
case re := <-readLoopResCh:
@@ -1205,7 +1212,6 @@
cc.forgetStreamID(cs.ID)
return nil, cs.getStartedWrite(), err
}
- bodyWritten = true
if d := cc.responseHeaderTimeout(); d != 0 {
timer := time.NewTimer(d)
defer timer.Stop()
diff --git a/http2/transport_test.go b/http2/transport_test.go
index fb4ec6c..d77c5bb 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -4861,3 +4861,48 @@
t.Fatal("expected closed")
}
}
+
+// Issue 31192: A failed request may be retried if the body has not been read
+// already. If the request body has started to be sent, one must wait until it
+// is completed.
+func TestTransportBodyRewindRace(t *testing.T) {
+ st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Connection", "close")
+ w.WriteHeader(http.StatusOK)
+ return
+ }, optOnlyServer)
+ defer st.Close()
+
+ tr := &http.Transport{
+ TLSClientConfig: tlsConfigInsecure,
+ MaxConnsPerHost: 1,
+ }
+ err := ConfigureTransport(tr)
+ if err != nil {
+ t.Fatal(err)
+ }
+ client := &http.Client{
+ Transport: tr,
+ }
+
+ const clients = 50
+
+ var wg sync.WaitGroup
+ wg.Add(clients)
+ for i := 0; i < clients; i++ {
+ req, err := http.NewRequest("POST", st.ts.URL, bytes.NewBufferString("abcdef"))
+ if err != nil {
+ t.Fatalf("unexpect new request error: %v", err)
+ }
+
+ go func() {
+ defer wg.Done()
+ res, err := client.Do(req)
+ if err == nil {
+ res.Body.Close()
+ }
+ }()
+ }
+
+ wg.Wait()
+}
