Skip to content

Commit 6e73886

Browse files
committed
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. 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]>
1 parent 8a7ee4c commit 6e73886

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
@@ -6441,10 +6441,11 @@ func TestErrorWriteLoopRace(t *testing.T) {
64416441
// Test that a new request which uses the connection of an active request
64426442
// cannot cause it to be canceled as well.
64436443
func TestCancelRequestWhenSharingConnection(t *testing.T) {
6444-
if testing.Short() {
6445-
t.Skip("skipping in short mode")
6446-
}
6444+
reqc := make(chan chan struct{}, 2)
64476445
ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, req *Request) {
6446+
ch := make(chan struct{}, 1)
6447+
reqc <- ch
6448+
<-ch
64486449
w.Header().Add("Content-Length", "0")
64496450
}))
64506451
defer ts.Close()
@@ -6456,34 +6457,58 @@ func TestCancelRequestWhenSharingConnection(t *testing.T) {
64566457

64576458
var wg sync.WaitGroup
64586459

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

6461-
for i := 0; i < 10; i++ {
6462-
wg.Add(1)
6463-
go func() {
6464-
defer wg.Done()
6465-
for ctx.Err() == nil {
6466-
reqctx, reqcancel := context.WithCancel(ctx)
6467-
go reqcancel()
6468-
req, _ := NewRequestWithContext(reqctx, "GET", ts.URL, nil)
6469-
res, err := client.Do(req)
6470-
if err == nil {
6471-
res.Body.Close()
6472-
}
6473-
}
6474-
}()
6475-
}
6483+
// Wait for the first request to receive a response and return the
6484+
// connection to the idle pool.
6485+
r1c := <-reqc
6486+
close(r1c)
6487+
idlec := <-putidlec
64766488

6477-
for ctx.Err() == nil {
6478-
req, _ := NewRequest("GET", ts.URL, nil)
6479-
if res, err := client.Do(req); err != nil {
6480-
t.Errorf("unexpected: %p %v", req, err)
6481-
break
6482-
} else {
6489+
wg.Add(1)
6490+
cancelctx, cancel := context.WithCancel(context.Background())
6491+
go func() {
6492+
defer wg.Done()
6493+
req, _ := NewRequestWithContext(cancelctx, "GET", ts.URL, nil)
6494+
res, err := client.Do(req)
6495+
if err == nil {
64836496
res.Body.Close()
64846497
}
6485-
}
6498+
if !errors.Is(err, context.Canceled) {
6499+
t.Errorf("request 2: got err %v, want Canceled", err)
6500+
}
6501+
}()
64866502

6503+
// Wait for the second request to arrive at the server, and then cancel
6504+
// the request context.
6505+
r2c := <-reqc
64876506
cancel()
6507+
6508+
// Give the cancelation a moment to take effect, and then unblock the first request.
6509+
time.Sleep(1 * time.Millisecond)
6510+
close(idlec)
6511+
6512+
close(r2c)
64886513
wg.Wait()
64896514
}

0 commit comments

Comments
 (0)