Skip to content

Commit

Permalink
http2: use synthetic time in TestIdleConnTimeout
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
neild committed Mar 20, 2024
1 parent d73acff commit d8870b0
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 37 deletions.
4 changes: 2 additions & 2 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
90 changes: 55 additions & 35 deletions http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}

Expand Down

0 comments on commit d8870b0

Please sign in to comment.