http2: speed up TestTransportHandlerBodyClose

Rewrite the slowest test in the http2 package.

This test was added in CL 23287 to verify two changes:
  - A server handler calling req.Body.Close does not
    kill the request stream.
  - A Transport does not leak a goroutine if a request body is
    still being written when the request stream is closed.

Split the test into two individual tests, one for the
server behavior and one for the transport.

Change-Id: I211f458e1001df435d00c2e1ebd7f3072e053c89
Reviewed-on: https://go-review.googlesource.com/c/net/+/700922
Reviewed-by: Nicholas Husin <[email protected]>
Reviewed-by: Nicholas Husin <[email protected]>
Auto-Submit: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: neild

http2/server_test.go

@@ -5066,3 +5066,60 @@ func testServerPingResponded(t testing.TB) {
 	st.advance(2 * time.Second)
 	st.wantIdle()
 }
+
+// golang.org/issue/15425: test that a handler closing the request
+// body doesn't terminate the stream to the peer. (It just stops
+// readability from the handler's side, and eventually the client
+// runs out of flow control tokens)
+func TestServerSendDataAfterRequestBodyClose(t *testing.T) {
+	synctestTest(t, testServerSendDataAfterRequestBodyClose)
+}
+func testServerSendDataAfterRequestBodyClose(t testing.TB) {
+	st := newServerTester(t, nil)
+	st.greet()
+
+	st.writeHeaders(HeadersFrameParam{
+		StreamID:      1,
+		BlockFragment: st.encodeHeader(),
+		EndStream:     false,
+		EndHeaders:    true,
+	})
+
+	// Handler starts writing the response body.
+	call := st.nextHandlerCall()
+	call.do(func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte("one"))
+		http.NewResponseController(w).Flush()
+	})
+	st.wantFrameType(FrameHeaders)
+	st.wantData(wantData{
+		streamID:  1,
+		endStream: false,
+		data:      []byte("one"),
+	})
+	st.wantIdle()
+
+	// Handler closes the request body.
+	// This is not observable by the client.
+	call.do(func(w http.ResponseWriter, req *http.Request) {
+		req.Body.Close()
+	})
+	st.wantIdle()
+
+	// The client can still send request data, which is discarded.
+	st.writeData(1, false, []byte("client-sent data"))
+	st.wantIdle()
+
+	// Handler can still write more response body,
+	// which is sent to the client.
+	call.do(func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte("two"))
+		http.NewResponseController(w).Flush()
+	})
+	st.wantData(wantData{
+		streamID:  1,
+		endStream: false,
+		data:      []byte("two"),
+	})
+	st.wantIdle()
+}

http2/transport_test.go

@@ -28,7 +28,6 @@ import (
 	"net/url"
 	"os"
 	"reflect"
-	"runtime"
 	"sort"
 	"strconv"
 	"strings"
@@ -2341,46 +2340,38 @@ func (b neverEnding) Read(p []byte) (int, error) {
 	return len(p), nil
 }
 
-// golang.org/issue/15425: test that a handler closing the request
-// body doesn't terminate the stream to the peer. (It just stops
-// readability from the handler's side, and eventually the client
-// runs out of flow control tokens)
-func TestTransportHandlerBodyClose(t *testing.T) {
-	const bodySize = 10 << 20
-	ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
-		r.Body.Close()
-		io.Copy(w, io.LimitReader(neverEnding('A'), bodySize))
-	})
-
-	tr := &Transport{TLSClientConfig: tlsConfigInsecure}
-	defer tr.CloseIdleConnections()
+// #15425: Transport goroutine leak while the transport is still trying to
+// write its body after the stream has completed.
+func TestTransportStreamEndsWhileBodyIsBeingWritten(t *testing.T) {
+	synctestTest(t, testTransportStreamEndsWhileBodyIsBeingWritten)
+}
+func testTransportStreamEndsWhileBodyIsBeingWritten(t testing.TB) {
+	body := "this is the client request body"
+	const windowSize = 10 // less than len(body)
 
-	g0 := runtime.NumGoroutine()
+	tc := newTestClientConn(t)
+	tc.greet(Setting{SettingInitialWindowSize, windowSize})
 
-	const numReq = 10
-	for i := 0; i < numReq; i++ {
-		req, err := http.NewRequest("POST", ts.URL, struct{ io.Reader }{io.LimitReader(neverEnding('A'), bodySize)})
-		if err != nil {
-			t.Fatal(err)
-		}
-		res, err := tr.RoundTrip(req)
-		if err != nil {
-			t.Fatal(err)
-		}
-		n, err := io.Copy(io.Discard, res.Body)
-		res.Body.Close()
-		if n != bodySize || err != nil {
-			t.Fatalf("req#%d: Copy = %d, %v; want %d, nil", i, n, err, bodySize)
-		}
-	}
-	tr.CloseIdleConnections()
+	// Client sends a request, and as much body as fits into the stream window.
+	req, _ := http.NewRequest("PUT", "https://dummy.tld/", strings.NewReader(body))
+	rt := tc.roundTrip(req)
+	tc.wantFrameType(FrameHeaders)
+	tc.wantData(wantData{
+		streamID:  rt.streamID(),
+		endStream: false,
+		size:      windowSize,
+	})
 
-	if !waitCondition(5*time.Second, 100*time.Millisecond, func() bool {
-		gd := runtime.NumGoroutine() - g0
-		return gd < numReq/2
-	}) {
-		t.Errorf("appeared to leak goroutines")
-	}
+	// Server responds without permitting the rest of the body to be sent.
+	tc.writeHeaders(HeadersFrameParam{
+		StreamID:   rt.streamID(),
+		EndHeaders: true,
+		EndStream:  true,
+		BlockFragment: tc.makeHeaderBlockFragment(
+			":status", "413",
+		),
+	})
+	rt.wantStatus(413)
 }
 
 // https://golang.org/issue/15930