respect minimum idle timeout of 3 PTOs (#3909)

connection.go

@@ -194,6 +194,7 @@ type connection struct {
 	versionNegotiated   bool
 	receivedFirstPacket bool
 
+	// the minimum of the max_idle_timeout values advertised by both endpoints
 	idleTimeout  time.Duration
 	creationTime time.Time
 	// The idle timeout is set based on the max of the time we received the last packet...
@@ -662,7 +663,7 @@ runLoop:
 		} else {
 			idleTimeoutStartTime := s.idleTimeoutStartTime()
 			if (!s.handshakeComplete && now.Sub(idleTimeoutStartTime) >= s.config.HandshakeIdleTimeout) ||
-				(s.handshakeComplete && now.Sub(idleTimeoutStartTime) >= s.idleTimeout) {
+				(s.handshakeComplete && now.After(s.nextIdleTimeoutTime())) {
 				s.destroyImpl(qerr.ErrIdleTimeout)
 				continue
 			}
@@ -720,13 +721,20 @@ func (s *connection) ConnectionState() ConnectionState {
 	return s.connState
 }
 
+// Time when the connection should time out
+func (s *connection) nextIdleTimeoutTime() time.Time {
+	idleTimeout := utils.Max(s.idleTimeout, s.rttStats.PTO(true)*3)
+	return s.idleTimeoutStartTime().Add(idleTimeout)
+}
+
 // Time when the next keep-alive packet should be sent.
 // It returns a zero time if no keep-alive should be sent.
 func (s *connection) nextKeepAliveTime() time.Time {
 	if s.config.KeepAlivePeriod == 0 || s.keepAlivePingSent || !s.firstAckElicitingPacketAfterIdleSentTime.IsZero() {
 		return time.Time{}
 	}
-	return s.lastPacketReceivedTime.Add(s.keepAliveInterval)
+	keepAliveInterval := utils.Max(s.keepAliveInterval, s.rttStats.PTO(true)*3/2)
+	return s.lastPacketReceivedTime.Add(keepAliveInterval)
 }
 
 func (s *connection) maybeResetTimer() {
@@ -740,7 +748,7 @@ func (s *connection) maybeResetTimer() {
 		if keepAliveTime := s.nextKeepAliveTime(); !keepAliveTime.IsZero() {
 			deadline = keepAliveTime
 		} else {
-			deadline = s.idleTimeoutStartTime().Add(s.idleTimeout)
+			deadline = s.nextIdleTimeoutTime()
 		}
 	}
 

connection_test.go

@@ -2137,6 +2137,20 @@ var _ = Describe("Connection", func() {
 			// don't EXPECT() any calls to mconn.Write()
 			time.Sleep(50 * time.Millisecond)
 		})
+
+		It("send PING as keep-alive earliest after 1.5 times the PTO", func() {
+			conn.config.KeepAlivePeriod = time.Microsecond
+			pto := conn.rttStats.PTO(true)
+			conn.lastPacketReceivedTime = time.Now()
+			sentPingTimeChan := make(chan time.Time)
+			packer.EXPECT().PackCoalescedPacket(false, gomock.Any(), conn.version).Do(func(bool, protocol.ByteCount, protocol.VersionNumber) (*coalescedPacket, error) {
+				sentPingTimeChan <- time.Now()
+				return nil, nil
+			})
+			runConn()
+			sentPingTime := <-sentPingTimeChan
+			Expect(sentPingTime.Sub(conn.lastPacketReceivedTime)).To(BeNumerically(">", pto*3/2))
+		})
 	})
 
 	Context("timeouts", func() {
@@ -2305,6 +2319,34 @@ var _ = Describe("Connection", func() {
 			conn.shutdown()
 			Eventually(conn.Context().Done()).Should(BeClosed())
 		})
+
+		It("time out earliest after 3 times the PTO", func() {
+			packer.EXPECT().PackCoalescedPacket(false, gomock.Any(), conn.version).AnyTimes()
+			connRunner.EXPECT().Retire(clientDestConnID)
+			connRunner.EXPECT().Remove(gomock.Any())
+			cryptoSetup.EXPECT().Close()
+			closeTimeChan := make(chan time.Time)
+			tracer.EXPECT().ClosedConnection(gomock.Any()).Do(func(e error) {
+				Expect(e).To(MatchError(&IdleTimeoutError{}))
+				closeTimeChan <- time.Now()
+			})
+			tracer.EXPECT().Close()
+			conn.idleTimeout = time.Millisecond
+			done := make(chan struct{})
+			pto := conn.rttStats.PTO(true)
+			go func() {
+				defer GinkgoRecover()
+				cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
+				cryptoSetup.EXPECT().GetSessionTicket().MaxTimes(1)
+				cryptoSetup.EXPECT().SetHandshakeConfirmed().MaxTimes(1)
+				close(conn.handshakeCompleteChan)
+				conn.run()
+				close(done)
+			}()
+			closeTime := <-closeTimeChan
+			Expect(closeTime.Sub(conn.lastPacketReceivedTime)).To(BeNumerically(">", pto*3))
+			Eventually(done).Should(BeClosed())
+		})
 	})
 
 	It("stores up to MaxConnUnprocessedPackets packets", func() {

integrationtests/self/timeout_test.go

@@ -105,7 +105,7 @@ var _ = Describe("Timeout tests", func() {
 	})
 
 	It("returns net.Error timeout errors when an idle timeout occurs", func() {
-		const idleTimeout = 100 * time.Millisecond
+		const idleTimeout = 500 * time.Millisecond
 
 		server, err := quic.ListenAddr(
 			"localhost:0",
@@ -173,7 +173,7 @@ var _ = Describe("Timeout tests", func() {
 		var idleTimeout time.Duration
 
 		BeforeEach(func() {
-			idleTimeout = scaleDuration(100 * time.Millisecond)
+			idleTimeout = scaleDuration(500 * time.Millisecond)
 		})
 
 		It("times out after inactivity", func() {
@@ -315,7 +315,7 @@ var _ = Describe("Timeout tests", func() {
 	})
 
 	It("does not time out if keepalive is set", func() {
-		const idleTimeout = 100 * time.Millisecond
+		const idleTimeout = 500 * time.Millisecond
 
 		server, err := quic.ListenAddr(
 			"localhost:0",