restart the idle timeout when sending the first retransmittable packet

packet_packer.go

@@ -47,6 +47,10 @@ func (p *packedPacket) EncryptionLevel() protocol.EncryptionLevel {
 	}
 }
 
+func (p *packedPacket) IsRetransmittable() bool {
+	return ackhandler.HasRetransmittableFrames(p.frames)
+}
+
 func (p *packedPacket) ToAckHandlerPacket() *ackhandler.Packet {
 	return &ackhandler.Packet{
 		PacketNumber:    p.header.PacketNumber,

session.go

@@ -124,8 +124,11 @@ type session struct {
 	receivedFirstPacket              bool
 	receivedFirstForwardSecurePacket bool
 
-	sessionCreationTime     time.Time
-	lastNetworkActivityTime time.Time
+	sessionCreationTime time.Time
+	// The idle timeout is set based on the max of the time we received the last packet...
+	lastPacketReceivedTime time.Time
+	// ... and the time we sent a new retransmittable packet after receiving a packet.
+	firstRetransmittablePacketAfterIdleSentTime time.Time
 	// pacingDeadline is the time when the next packet should be sent
 	pacingDeadline time.Time
 
@@ -319,7 +322,7 @@ func (s *session) postSetup() error {
 
 	s.timer = utils.NewTimer()
 	now := time.Now()
-	s.lastNetworkActivityTime = now
+	s.lastPacketReceivedTime = now
 	s.sessionCreationTime = now
 
 	s.windowUpdateQueue = newWindowUpdateQueue(s.streamsMap, s.connFlowController, s.framer.QueueControlFrame)
@@ -396,7 +399,7 @@ runLoop:
 		if s.pacingDeadline.IsZero() { // the timer didn't have a pacing deadline set
 			pacingDeadline = s.sentPacketHandler.TimeUntilSend()
 		}
-		if s.config.KeepAlive && !s.keepAlivePingSent && s.handshakeComplete && time.Since(s.lastNetworkActivityTime) >= s.peerParams.IdleTimeout/2 {
+		if s.config.KeepAlive && !s.keepAlivePingSent && s.handshakeComplete && s.firstRetransmittablePacketAfterIdleSentTime.IsZero() && time.Since(s.lastPacketReceivedTime) >= s.peerParams.IdleTimeout/2 {
 			// send a PING frame since there is no activity in the session
 			s.logger.Debugf("Sending a keep-alive ping to keep the connection alive.")
 			s.framer.QueueControlFrame(&wire.PingFrame{})
@@ -413,7 +416,7 @@ runLoop:
 			s.destroy(qerr.TimeoutError("Handshake did not complete in time"))
 			continue
 		}
-		if s.handshakeComplete && now.Sub(s.lastNetworkActivityTime) >= s.config.IdleTimeout {
+		if s.handshakeComplete && now.Sub(s.idleTimeoutStartTime()) >= s.config.IdleTimeout {
 			s.destroy(qerr.TimeoutError("No recent network activity"))
 			continue
 		}
@@ -441,9 +444,9 @@ func (s *session) ConnectionState() ConnectionState {
 func (s *session) maybeResetTimer() {
 	var deadline time.Time
 	if s.config.KeepAlive && s.handshakeComplete && !s.keepAlivePingSent {
-		deadline = s.lastNetworkActivityTime.Add(s.peerParams.IdleTimeout / 2)
+		deadline = s.idleTimeoutStartTime().Add(s.peerParams.IdleTimeout / 2)
 	} else {
-		deadline = s.lastNetworkActivityTime.Add(s.config.IdleTimeout)
+		deadline = s.idleTimeoutStartTime().Add(s.config.IdleTimeout)
 	}
 
 	if ackAlarm := s.receivedPacketHandler.GetAlarmTimeout(); !ackAlarm.IsZero() {
@@ -463,6 +466,10 @@ func (s *session) maybeResetTimer() {
 	s.timer.Reset(deadline)
 }
 
+func (s *session) idleTimeoutStartTime() time.Time {
+	return utils.MaxTime(s.lastPacketReceivedTime, s.firstRetransmittablePacketAfterIdleSentTime)
+}
+
 func (s *session) handleHandshakeComplete() {
 	s.handshakeComplete = true
 	s.handshakeCompleteChan = nil // prevent this case from ever being selected again
@@ -622,7 +629,8 @@ func (s *session) handleUnpackedPacket(packet *unpackedPacket, rcvTime time.Time
 	}
 
 	s.receivedFirstPacket = true
-	s.lastNetworkActivityTime = rcvTime
+	s.lastPacketReceivedTime = rcvTime
+	s.firstRetransmittablePacketAfterIdleSentTime = time.Time{}
 	s.keepAlivePingSent = false
 
 	// The client completes the handshake first (after sending the CFIN).
@@ -810,7 +818,7 @@ func (s *session) handlePathChallengeFrame(frame *wire.PathChallengeFrame) {
 }
 
 func (s *session) handleAckFrame(frame *wire.AckFrame, pn protocol.PacketNumber, encLevel protocol.EncryptionLevel) error {
-	if err := s.sentPacketHandler.ReceivedAck(frame, pn, encLevel, s.lastNetworkActivityTime); err != nil {
+	if err := s.sentPacketHandler.ReceivedAck(frame, pn, encLevel, s.lastPacketReceivedTime); err != nil {
 		return err
 	}
 	if encLevel == protocol.Encryption1RTT {
@@ -1128,6 +1136,9 @@ func (s *session) sendPacket() (bool, error) {
 
 func (s *session) sendPackedPacket(packet *packedPacket) error {
 	defer packet.buffer.Release()
+	if s.firstRetransmittablePacketAfterIdleSentTime.IsZero() && packet.IsRetransmittable() {
+		s.firstRetransmittablePacketAfterIdleSentTime = time.Now()
+	}
 	s.logPacket(packet)
 	return s.conn.Write(packet.raw)
 }

session_test.go

@@ -1232,10 +1232,10 @@ var _ = Describe("Session", func() {
 			sess.peerParams = &handshake.TransportParameters{IdleTimeout: remoteIdleTimeout}
 		})
 
-		It("sends a PING", func() {
+		It("sends a PING as a keep-alive", func() {
 			sess.handshakeComplete = true
 			sess.config.KeepAlive = true
-			sess.lastNetworkActivityTime = time.Now().Add(-remoteIdleTimeout / 2)
+			sess.lastPacketReceivedTime = time.Now().Add(-remoteIdleTimeout / 2)
 			sent := make(chan struct{})
 			packer.EXPECT().PackPacket().Do(func() (*packedPacket, error) {
 				close(sent)
@@ -1261,7 +1261,7 @@ var _ = Describe("Session", func() {
 		It("doesn't send a PING packet if keep-alive is disabled", func() {
 			sess.handshakeComplete = true
 			sess.config.KeepAlive = false
-			sess.lastNetworkActivityTime = time.Now().Add(-remoteIdleTimeout / 2)
+			sess.lastPacketReceivedTime = time.Now().Add(-remoteIdleTimeout / 2)
 			done := make(chan struct{})
 			go func() {
 				defer GinkgoRecover()
@@ -1282,7 +1282,7 @@ var _ = Describe("Session", func() {
 		It("doesn't send a PING if the handshake isn't completed yet", func() {
 			sess.handshakeComplete = false
 			sess.config.KeepAlive = true
-			sess.lastNetworkActivityTime = time.Now().Add(-remoteIdleTimeout / 2)
+			sess.lastPacketReceivedTime = time.Now().Add(-remoteIdleTimeout / 2)
 			done := make(chan struct{})
 			go func() {
 				defer GinkgoRecover()
@@ -1309,7 +1309,7 @@ var _ = Describe("Session", func() {
 		It("times out due to no network activity", func() {
 			sessionRunner.EXPECT().removeConnectionID(gomock.Any())
 			sess.handshakeComplete = true
-			sess.lastNetworkActivityTime = time.Now().Add(-time.Hour)
+			sess.lastPacketReceivedTime = time.Now().Add(-time.Hour)
 			done := make(chan struct{})
 			cryptoSetup.EXPECT().Close()
 			go func() {
@@ -1345,7 +1345,7 @@ var _ = Describe("Session", func() {
 
 		It("does not use the idle timeout before the handshake complete", func() {
 			sess.config.IdleTimeout = 9999 * time.Second
-			sess.lastNetworkActivityTime = time.Now().Add(-time.Minute)
+			sess.lastPacketReceivedTime = time.Now().Add(-time.Minute)
 			packer.EXPECT().PackConnectionClose(gomock.Any()).DoAndReturn(func(f *wire.ConnectionCloseFrame) (*packedPacket, error) {
 				Expect(f.ErrorCode).To(Equal(qerr.NoError))
 				return &packedPacket{}, nil
@@ -1384,6 +1384,25 @@ var _ = Describe("Session", func() {
 			}()
 			Eventually(done).Should(BeClosed())
 		})
+
+		It("doesn't time out when it just sent a packet", func() {
+			sess.handshakeComplete = true
+			sess.lastPacketReceivedTime = time.Now().Add(-time.Hour)
+			sess.firstRetransmittablePacketAfterIdleSentTime = time.Now().Add(-time.Second)
+			sess.config.IdleTimeout = 30 * time.Second
+			go func() {
+				defer GinkgoRecover()
+				cryptoSetup.EXPECT().RunHandshake().Do(func() { <-sess.Context().Done() })
+				sess.run()
+			}()
+			Consistently(sess.Context().Done()).ShouldNot(BeClosed())
+			// make the go routine return
+			packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
+			sessionRunner.EXPECT().retireConnectionID(gomock.Any())
+			cryptoSetup.EXPECT().Close()
+			sess.Close()
+			Eventually(sess.Context().Done()).Should(BeClosed())
+		})
 	})
 
 	It("stores up to MaxSessionUnprocessedPackets packets", func(done Done) {