Skip to content

Commit a6ca6d9

Browse files
neildheschi
authored andcommitted
[release-branch.go1.16] net/http: speed up and deflake TestCancelRequestWhenSharingConnection
This test made many requests over the same connection for 10 seconds, trusting that this will exercise the request cancelation race from #41600. Change the test to exhibit the specific race in a targeted fashion with only two requests. Fixes #47535. Updates #41600. Updates #47016. Change-Id: If99c9b9331ff645f6bb67fe9fb79b8aab8784710 Reviewed-on: https://go-review.googlesource.com/c/go/+/339594 Trust: Damien Neil <[email protected]> Run-TryBot: Damien Neil <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> (cherry picked from commit 6e73886) Reviewed-on: https://go-review.googlesource.com/c/go/+/339830
1 parent 16ab7e4 commit a6ca6d9

File tree

1 file changed

+51
-26
lines changed

1 file changed

+51
-26
lines changed

src/net/http/transport_test.go

+51-26
Original file line numberDiff line numberDiff line change
@@ -6433,10 +6433,11 @@ func TestErrorWriteLoopRace(t *testing.T) {
64336433
// Test that a new request which uses the connection of an active request
64346434
// cannot cause it to be canceled as well.
64356435
func TestCancelRequestWhenSharingConnection(t *testing.T) {
6436-
if testing.Short() {
6437-
t.Skip("skipping in short mode")
6438-
}
6436+
reqc := make(chan chan struct{}, 2)
64396437
ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, req *Request) {
6438+
ch := make(chan struct{}, 1)
6439+
reqc <- ch
6440+
<-ch
64406441
w.Header().Add("Content-Length", "0")
64416442
}))
64426443
defer ts.Close()
@@ -6448,34 +6449,58 @@ func TestCancelRequestWhenSharingConnection(t *testing.T) {
64486449

64496450
var wg sync.WaitGroup
64506451

6451-
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
6452+
wg.Add(1)
6453+
putidlec := make(chan chan struct{})
6454+
go func() {
6455+
defer wg.Done()
6456+
ctx := httptrace.WithClientTrace(context.Background(), &httptrace.ClientTrace{
6457+
PutIdleConn: func(error) {
6458+
// Signal that the idle conn has been returned to the pool,
6459+
// and wait for the order to proceed.
6460+
ch := make(chan struct{})
6461+
putidlec <- ch
6462+
<-ch
6463+
},
6464+
})
6465+
req, _ := NewRequestWithContext(ctx, "GET", ts.URL, nil)
6466+
res, err := client.Do(req)
6467+
if err == nil {
6468+
res.Body.Close()
6469+
}
6470+
if err != nil {
6471+
t.Errorf("request 1: got err %v, want nil", err)
6472+
}
6473+
}()
64526474

6453-
for i := 0; i < 10; i++ {
6454-
wg.Add(1)
6455-
go func() {
6456-
defer wg.Done()
6457-
for ctx.Err() == nil {
6458-
reqctx, reqcancel := context.WithCancel(ctx)
6459-
go reqcancel()
6460-
req, _ := NewRequestWithContext(reqctx, "GET", ts.URL, nil)
6461-
res, err := client.Do(req)
6462-
if err == nil {
6463-
res.Body.Close()
6464-
}
6465-
}
6466-
}()
6467-
}
6475+
// Wait for the first request to receive a response and return the
6476+
// connection to the idle pool.
6477+
r1c := <-reqc
6478+
close(r1c)
6479+
idlec := <-putidlec
64686480

6469-
for ctx.Err() == nil {
6470-
req, _ := NewRequest("GET", ts.URL, nil)
6471-
if res, err := client.Do(req); err != nil {
6472-
t.Errorf("unexpected: %p %v", req, err)
6473-
break
6474-
} else {
6481+
wg.Add(1)
6482+
cancelctx, cancel := context.WithCancel(context.Background())
6483+
go func() {
6484+
defer wg.Done()
6485+
req, _ := NewRequestWithContext(cancelctx, "GET", ts.URL, nil)
6486+
res, err := client.Do(req)
6487+
if err == nil {
64756488
res.Body.Close()
64766489
}
6477-
}
6490+
if !errors.Is(err, context.Canceled) {
6491+
t.Errorf("request 2: got err %v, want Canceled", err)
6492+
}
6493+
}()
64786494

6495+
// Wait for the second request to arrive at the server, and then cancel
6496+
// the request context.
6497+
r2c := <-reqc
64796498
cancel()
6499+
6500+
// Give the cancelation a moment to take effect, and then unblock the first request.
6501+
time.Sleep(1 * time.Millisecond)
6502+
close(idlec)
6503+
6504+
close(r2c)
64806505
wg.Wait()
64816506
}

0 commit comments

Comments
 (0)