use a monotonous timer for the connection (quic-go#3570)

There's no point in having the timer fire multiple times for the same
timestamp. By using a monotonuos timer we avoid busy-looping in cases where the
timer fires, but we can't actually send a packet.

connection.go

@@ -209,7 +209,7 @@ type connection struct {
 
 	peerParams *wire.TransportParameters
 
-	timer *utils.Timer
+	timer connectionTimer
 	// keepAlivePingSent stores whether a keep alive PING is in flight.
 	// It is reset as soon as we receive a packet from the peer.
 	keepAlivePingSent bool
@@ -223,10 +223,9 @@ type connection struct {
 }
 
 var (
-	_                       Connection      = &connection{}
-	_                       EarlyConnection = &connection{}
-	_                       streamSender    = &connection{}
-	deadlineSendImmediately                 = time.Time{}.Add(42 * time.Millisecond) // any value > time.Time{} and before time.Now() is fine
+	_ Connection      = &connection{}
+	_ EarlyConnection = &connection{}
+	_ streamSender    = &connection{}
 )
 
 var newConnection = func(
@@ -548,7 +547,7 @@ func (s *connection) preSetup() {
 func (s *connection) run() error {
 	defer s.ctxCancel()
 
-	s.timer = utils.NewTimer()
+	s.timer = *newTimer()
 
 	handshaking := make(chan struct{})
 	go func() {
@@ -765,17 +764,12 @@ func (s *connection) maybeResetTimer() {
 		}
 	}
 
-	if ackAlarm := s.receivedPacketHandler.GetAlarmTimeout(); !ackAlarm.IsZero() {
-		deadline = utils.MinTime(deadline, ackAlarm)
-	}
-	if lossTime := s.sentPacketHandler.GetLossDetectionTimeout(); !lossTime.IsZero() {
-		deadline = utils.MinTime(deadline, lossTime)
-	}
-	if !s.pacingDeadline.IsZero() {
-		deadline = utils.MinTime(deadline, s.pacingDeadline)
-	}
-
-	s.timer.Reset(deadline)
+	s.timer.SetTimer(
+		deadline,
+		s.receivedPacketHandler.GetAlarmTimeout(),
+		s.sentPacketHandler.GetLossDetectionTimeout(),
+		s.pacingDeadline,
+	)
 }
 
 func (s *connection) idleTimeoutStartTime() time.Time {
@@ -1678,7 +1672,7 @@ func (s *connection) sendPackets() error {
 			}
 			// We can at most send a single ACK only packet.
 			// There will only be a new ACK after receiving new packets.
-			// SendAck is only returned when we're congestion limited, so we don't need to set the pacingt timer.
+			// SendAck is only returned when we're congestion limited, so we don't need to set the pacinggs timer.
 			return s.maybeSendAckOnlyPacket()
 		case ackhandler.SendPTOInitial:
 			if err := s.sendProbePacket(protocol.EncryptionInitial); err != nil {

connection_timer.go

@@ -0,0 +1,51 @@
+package quic
+
+import (
+	"time"
+
+	"github.com/lucas-clemente/quic-go/internal/utils"
+)
+
+var deadlineSendImmediately = time.Time{}.Add(42 * time.Millisecond) // any value > time.Time{} and before time.Now() is fine
+
+type connectionTimer struct {
+	timer *utils.Timer
+	last  time.Time
+}
+
+func newTimer() *connectionTimer {
+	return &connectionTimer{timer: utils.NewTimer()}
+}
+
+func (t *connectionTimer) SetRead() {
+	if deadline := t.timer.Deadline(); deadline != deadlineSendImmediately {
+		t.last = deadline
+	}
+	t.timer.SetRead()
+}
+
+func (t *connectionTimer) Chan() <-chan time.Time {
+	return t.timer.Chan()
+}
+
+// SetTimer resets the timer.
+// It makes sure that the deadline is strictly increasing.
+// This prevents busy-looping in cases where the timer fires, but we can't actually send out a packet.
+// This doesn't apply to the pacing deadline, which can be set multiple times to deadlineSendImmediately.
+func (t *connectionTimer) SetTimer(idleTimeoutOrKeepAlive, ackAlarm, lossTime, pacing time.Time) {
+	deadline := idleTimeoutOrKeepAlive
+	if !ackAlarm.IsZero() && ackAlarm.Before(deadline) && ackAlarm.After(t.last) {
+		deadline = ackAlarm
+	}
+	if !lossTime.IsZero() && lossTime.Before(deadline) && lossTime.After(t.last) {
+		deadline = lossTime
+	}
+	if !pacing.IsZero() && pacing.Before(deadline) {
+		deadline = pacing
+	}
+	t.timer.Reset(deadline)
+}
+
+func (t *connectionTimer) Stop() {
+	t.timer.Stop()
+}

connection_timer_test.go

@@ -0,0 +1,62 @@
+package quic
+
+import (
+	"time"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func (t *connectionTimer) Deadline() time.Time { return t.timer.Deadline() }
+
+var _ = Describe("Timer", func() {
+	It("sets an idle timeout", func() {
+		now := time.Now()
+		t := newTimer()
+		t.SetTimer(now.Add(time.Hour), time.Time{}, time.Time{}, time.Time{})
+		Expect(t.Deadline()).To(Equal(now.Add(time.Hour)))
+	})
+
+	It("sets an ACK timer", func() {
+		now := time.Now()
+		t := newTimer()
+		t.SetTimer(now.Add(time.Hour), now.Add(time.Minute), time.Time{}, time.Time{})
+		Expect(t.Deadline()).To(Equal(now.Add(time.Minute)))
+	})
+
+	It("sets a loss timer", func() {
+		now := time.Now()
+		t := newTimer()
+		t.SetTimer(now.Add(time.Hour), now.Add(time.Minute), now.Add(time.Second), time.Time{})
+		Expect(t.Deadline()).To(Equal(now.Add(time.Second)))
+	})
+
+	It("sets a pacing timer", func() {
+		now := time.Now()
+		t := newTimer()
+		t.SetTimer(now.Add(time.Hour), now.Add(time.Minute), now.Add(time.Second), now.Add(time.Millisecond))
+		Expect(t.Deadline()).To(Equal(now.Add(time.Millisecond)))
+	})
+
+	It("doesn't reset to an earlier time", func() {
+		now := time.Now()
+		t := newTimer()
+		t.SetTimer(now.Add(time.Hour), now.Add(time.Minute), time.Time{}, time.Time{})
+		Expect(t.Deadline()).To(Equal(now.Add(time.Minute)))
+		t.SetRead()
+
+		t.SetTimer(now.Add(time.Hour), now.Add(time.Minute), time.Time{}, time.Time{})
+		Expect(t.Deadline()).To(Equal(now.Add(time.Hour)))
+	})
+
+	It("allows the pacing timer to be set to send immediately", func() {
+		now := time.Now()
+		t := newTimer()
+		t.SetTimer(now.Add(time.Hour), now.Add(time.Minute), time.Time{}, time.Time{})
+		Expect(t.Deadline()).To(Equal(now.Add(time.Minute)))
+		t.SetRead()
+
+		t.SetTimer(now.Add(time.Hour), now.Add(time.Minute), time.Time{}, deadlineSendImmediately)
+		Expect(t.Deadline()).To(Equal(deadlineSendImmediately))
+	})
+})

internal/utils/timer.go

@@ -47,6 +47,10 @@ func (t *Timer) SetRead() {
 	t.read = true
 }
 
+func (t *Timer) Deadline() time.Time {
+	return t.deadline
+}
+
 // Stop stops the timer
 func (t *Timer) Stop() {
 	t.t.Stop()

internal/utils/timer_test.go

@@ -21,6 +21,14 @@ var _ = Describe("Timer", func() {
 		Eventually(t.Chan()).Should(Receive())
 	})
 
+	It("returns the deadline", func() {
+		t := NewTimer()
+		deadline := time.Now().Add(d)
+		t.Reset(deadline)
+		Expect(t.Deadline()).To(Equal(deadline))
+		Eventually(t.Chan()).Should(Receive())
+	})
+
 	It("works multiple times with reading", func() {
 		t := NewTimer()
 		for i := 0; i < 10; i++ {