diff --git a/connection.go b/connection.go index 5aa8d97f..c544b591 100644 --- a/connection.go +++ b/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() } } diff --git a/connection_test.go b/connection_test.go index c8e16bfd..177aa441 100644 --- a/connection_test.go +++ b/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() { diff --git a/integrationtests/self/timeout_test.go b/integrationtests/self/timeout_test.go index 358ff1e2..c5ec46d4 100644 --- a/integrationtests/self/timeout_test.go +++ b/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",