From 331a10ca0734203bea6442b8fd388386332451e3 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Thu, 21 Nov 2019 23:23:37 -0800 Subject: [PATCH] User time.AfterFunc to remove the need of explicitly managing goroutines The race in https://github.com/golang/go/issues/11513 does not affect time.AfterFunc, because the timer returned by time.AfterFunc() does not have the buffered channel at all. --- http2/transport.go | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/http2/transport.go b/http2/transport.go index 8b1e6fb623..0402766b21 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -113,10 +113,12 @@ type Transport struct { // if response is not received within PingTimeout. // 0 means no periodic pings. Defaults to 0. PingPeriod time.Duration + // PingTimeout is the timeout after which the connection will be closed // if a response to Ping is not received. // 0 means no periodic pings. Defaults to 0. PingTimeout time.Duration + // ReadIdleTimeout is the timeout after which the periodic ping for // connection health check will begin if no frame is received on the // connection. @@ -148,6 +150,14 @@ func (t *Transport) disableCompression() bool { return t.DisableCompression || (t.t1 != nil && t.t1.DisableCompression) } +func (t *Transport) readIdleTimeout() time.Duration { + to := t.ReadIdleTimeout + if to == 0 { + to = 60 * time.Second + } + return to +} + // ConfigureTransport configures a net/http HTTP/1 Transport to use HTTP/2. // It returns an error if t1 has already been HTTP/2-enabled. func ConfigureTransport(t1 *http.Transport) error { @@ -1775,42 +1785,17 @@ func (rl *clientConnReadLoop) cleanup() { cc.mu.Unlock() } -type frameAndError struct { - f Frame - err error -} - -func nonBlockingReadFrame(fr *Framer) chan frameAndError { - feCh := make(chan frameAndError) - go func() { - f, err := fr.ReadFrame() - feCh <- frameAndError{f: f, err: err} - }() - return feCh -} - func (rl *clientConnReadLoop) run() error { cc := rl.cc rl.closeWhenIdle = cc.t.disableKeepAlives() || cc.singleUse gotReply := false // ever saw a HEADERS reply gotSettings := false + to := cc.t.readIdleTimeout() + t := time.AfterFunc(to, cc.startHealthCheck) for { - var fe frameAndError - feCh := nonBlockingReadFrame(cc.fr) - to := cc.t.ReadIdleTimeout - if to == 0 { - to = 60 * time.Second - } - readIdleTimer := time.NewTimer(to) - select { - case fe = <-feCh: - cc.stopHealthCheck() - readIdleTimer.Stop() - case <-readIdleTimer.C: - cc.startHealthCheck() - fe = <-feCh - } - f, err := fe.f, fe.err + f, err := cc.fr.ReadFrame() + t.Reset(to) + cc.stopHealthCheck() if err != nil { cc.vlogf("http2: Transport readFrame error on conn %p: (%T) %v", cc, err, err) }