http2: use synthetic time in TestIdleConnTimeout

neild · neild · commit d8870b0bf2f2 · 2024-03-20T17:25:32.000Z
Rewrite TestIdleConnTimeout to use the new synthetic time and synchronization test facilities, rather than using real time and sleeps. Reduces the test time from 20 seconds to 0. Reduces all package tests on my laptop from 32 seconds to 12. Change-Id: I33838488168450a7acd6a462777b5a4caf7f5307 Reviewed-on: https://go-review.googlesource.com/c/net/+/572379 Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/http2/transport.go b/http2/transport.go
@@ -310,7 +310,7 @@ type ClientConn struct {
 	readerErr  error         // set before readerDone is closed
 
 	idleTimeout time.Duration // or 0 for never
-	idleTimer   *time.Timer
+	idleTimer   timer
 
 	mu              sync.Mutex // guards following
 	cond            *sync.Cond // hold mu; broadcast on flow/closed changes
@@ -828,7 +828,7 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool, hooks *testSyncHoo
 	}
 	if d := t.idleConnTimeout(); d != 0 {
 		cc.idleTimeout = d
-		cc.idleTimer = time.AfterFunc(d, cc.onIdleTimeout)
+		cc.idleTimer = cc.afterFunc(d, cc.onIdleTimeout)
 	}
 	if VerboseLogs {
 		t.vlogf("http2: Transport creating client conn %p to %v", cc, c.RemoteAddr())
diff --git a/http2/transport_test.go b/http2/transport_test.go
@@ -97,63 +97,83 @@ func startH2cServer(t *testing.T) net.Listener {
 
 func TestIdleConnTimeout(t *testing.T) {
 	for _, test := range []struct {
+		name            string
 		idleConnTimeout time.Duration
 		wait            time.Duration
 		baseTransport   *http.Transport
-		wantConns       int32
+		wantNewConn     bool
 	}{{
+		name:            "NoExpiry",
 		idleConnTimeout: 2 * time.Second,
 		wait:            1 * time.Second,
 		baseTransport:   nil,
-		wantConns:       1,
+		wantNewConn:     false,
 	}, {
+		name:            "H2TransportTimeoutExpires",
 		idleConnTimeout: 1 * time.Second,
 		wait:            2 * time.Second,
 		baseTransport:   nil,
-		wantConns:       5,
+		wantNewConn:     true,
 	}, {
+		name:            "H1TransportTimeoutExpires",
 		idleConnTimeout: 0 * time.Second,
 		wait:            1 * time.Second,
 		baseTransport: &http.Transport{
 			IdleConnTimeout: 2 * time.Second,
 		},
-		wantConns: 1,
+		wantNewConn: false,
 	}} {
-		var gotConns int32
-
-		st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
-			io.WriteString(w, r.RemoteAddr)
-		}, optOnlyServer)
-		defer st.Close()
+		t.Run(test.name, func(t *testing.T) {
+			tt := newTestTransport(t, func(tr *Transport) {
+				tr.IdleConnTimeout = test.idleConnTimeout
+			})
+			var tc *testClientConn
+			for i := 0; i < 3; i++ {
+				req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
+				rt := tt.roundTrip(req)
+
+				// This request happens on a new conn if it's the first request
+				// (and there is no cached conn), or if the test timeout is long
+				// enough that old conns are being closed.
+				wantConn := i == 0 || test.wantNewConn
+				if has := tt.hasConn(); has != wantConn {
+					t.Fatalf("request %v: hasConn=%v, want %v", i, has, wantConn)
+				}
+				if wantConn {
+					tc = tt.getConn()
+					// Read client's SETTINGS and first WINDOW_UPDATE,
+					// send our SETTINGS.
+					tc.wantFrameType(FrameSettings)
+					tc.wantFrameType(FrameWindowUpdate)
+					tc.writeSettings()
+				}
+				if tt.hasConn() {
+					t.Fatalf("request %v: Transport has more than one conn", i)
+				}
 
-		tr := &Transport{
-			IdleConnTimeout: test.idleConnTimeout,
-			TLSClientConfig: tlsConfigInsecure,
-		}
-		defer tr.CloseIdleConnections()
+				// Respond to the client's request.
+				hf := testClientConnReadFrame[*MetaHeadersFrame](tc)
+				tc.writeHeaders(HeadersFrameParam{
+					StreamID:   hf.StreamID,
+					EndHeaders: true,
+					EndStream:  true,
+					BlockFragment: tc.makeHeaderBlockFragment(
+						":status", "200",
+					),
+				})
+				rt.wantStatus(200)
 
-		for i := 0; i < 5; i++ {
-			req, _ := http.NewRequest("GET", st.ts.URL, http.NoBody)
-			trace := &httptrace.ClientTrace{
-				GotConn: func(connInfo httptrace.GotConnInfo) {
-					if !connInfo.Reused {
-						atomic.AddInt32(&gotConns, 1)
-					}
-				},
-			}
-			req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace))
+				// If this was a newly-accepted conn, read the SETTINGS ACK.
+				if wantConn {
+					tc.wantFrameType(FrameSettings) // ACK to our settings
+				}
 
-			_, err := tr.RoundTrip(req)
-			if err != nil {
-				t.Fatalf("%v", err)
+				tt.advance(test.wait)
+				if got, want := tc.netConnClosed, test.wantNewConn; got != want {
+					t.Fatalf("after waiting %v, conn closed=%v; want %v", test.wait, got, want)
+				}
 			}
-
-			<-time.After(test.wait)
-		}
-
-		if gotConns != test.wantConns {
-			t.Errorf("incorrect gotConns: %d != %d", gotConns, test.wantConns)
-		}
+		})
 	}
 }
 
