From 079279b9cf4cb5dafc8b7f673a2e7e47a4b6a06e Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Wed, 8 Jan 2020 11:09:45 +0700
Subject: [PATCH] fix mismatching expectation of the keep alive timer

session.maybeResetTimer() and session.run() were using slightly
different definitions of when a keep-alive PING should be sent. Under
certain conditions, this would make us repeatedly set a timer for the
keep-alive, but on timer expiration no keep-alive would be sent.
---
 session.go      | 26 ++++++++++++++++++++------
 session_test.go |  2 ++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/session.go b/session.go
index 0bbabad6..e34c8973 100644
--- a/session.go
+++ b/session.go
@@ -524,11 +524,17 @@ 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 && s.firstAckElicitingPacketAfterIdleSentTime.IsZero() && time.Since(s.lastPacketReceivedTime) >= s.keepAliveInterval/2 {
+		if keepAliveTime := s.nextKeepAliveTime(); !keepAliveTime.IsZero() && !now.Before(keepAliveTime) {
 			// 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.logger.Debugf("Sending a keep-alive PING to keep the connection alive.")
 			s.framer.QueueControlFrame(&wire.PingFrame{})
 			s.keepAlivePingSent = true
+		} else if !s.handshakeComplete && now.Sub(s.sessionCreationTime) >= s.config.HandshakeTimeout {
+			s.destroyImpl(qerr.TimeoutError("Handshake did not complete in time"))
+			continue
+		} else if s.handshakeComplete && now.Sub(s.idleTimeoutStartTime()) >= s.idleTimeout {
+			s.destroyImpl(qerr.TimeoutError("No recent network activity"))
+			continue
 		} else if !pacingDeadline.IsZero() && now.Before(pacingDeadline) {
 			// If we get to this point before the pacing deadline, we should wait until that deadline.
 			// This can happen when scheduleSending is called, or a packet is received.
@@ -575,14 +581,22 @@ func (s *session) ConnectionState() tls.ConnectionState {
 	return s.cryptoStreamHandler.ConnectionState()
 }
 
+// Time when the next keep-alive packet should be sent.
+// It returns a zero time if no keep-alive should be sent.
+func (s *session) nextKeepAliveTime() time.Time {
+	if !s.config.KeepAlive || s.keepAlivePingSent || s.firstAckElicitingPacketAfterIdleSentTime.IsZero() {
+		return time.Time{}
+	}
+	return s.lastPacketReceivedTime.Add(s.keepAliveInterval / 2)
+}
+
 func (s *session) maybeResetTimer() {
 	var deadline time.Time
 	if !s.handshakeComplete {
-		handshakeDeadline := s.sessionCreationTime.Add(s.config.HandshakeTimeout)
-		deadline = handshakeDeadline
+		deadline = s.sessionCreationTime.Add(s.config.HandshakeTimeout)
 	} else {
-		if s.config.KeepAlive && !s.keepAlivePingSent {
-			deadline = s.idleTimeoutStartTime().Add(s.keepAliveInterval / 2)
+		if keepAliveTime := s.nextKeepAliveTime(); !keepAliveTime.IsZero() {
+			deadline = keepAliveTime
 		} else {
 			deadline = s.idleTimeoutStartTime().Add(s.idleTimeout)
 		}
diff --git a/session_test.go b/session_test.go
index cdc5ef24..9a842e1a 100644
--- a/session_test.go
+++ b/session_test.go
@@ -1382,6 +1382,7 @@ var _ = Describe("Session", func() {
 		It("sends a PING as a keep-alive after half the idle timeout", func() {
 			setRemoteIdleTimeout(5 * time.Second)
 			sess.lastPacketReceivedTime = time.Now().Add(-5 * time.Second / 2)
+			sess.firstAckElicitingPacketAfterIdleSentTime = time.Now().Add(-5 * time.Second / 2)
 			sent := make(chan struct{})
 			packer.EXPECT().PackPacket().Do(func() (*packedPacket, error) {
 				close(sent)
@@ -1395,6 +1396,7 @@ var _ = Describe("Session", func() {
 			sess.config.MaxIdleTimeout = time.Hour
 			setRemoteIdleTimeout(time.Hour)
 			sess.lastPacketReceivedTime = time.Now().Add(-protocol.MaxKeepAliveInterval).Add(-time.Millisecond)
+			sess.firstAckElicitingPacketAfterIdleSentTime = time.Now().Add(-protocol.MaxKeepAliveInterval).Add(-time.Millisecond)
 			sent := make(chan struct{})
 			packer.EXPECT().PackPacket().Do(func() (*packedPacket, error) {
 				close(sent)