From 63f911222e782cab99be91785ba15b5daa26c509 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 11 Oct 2022 15:36:10 +0300 Subject: [PATCH] use a monotonous timer for the connection (#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 | 30 +++++++---------- connection_timer.go | 51 +++++++++++++++++++++++++++++ connection_timer_test.go | 62 ++++++++++++++++++++++++++++++++++++ internal/utils/timer.go | 4 +++ internal/utils/timer_test.go | 8 +++++ 5 files changed, 137 insertions(+), 18 deletions(-) create mode 100644 connection_timer.go create mode 100644 connection_timer_test.go diff --git a/connection.go b/connection.go index 015d98adcbe..741be429db8 100644 --- a/connection.go +++ b/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 { diff --git a/connection_timer.go b/connection_timer.go new file mode 100644 index 00000000000..1c13cfb6c01 --- /dev/null +++ b/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() +} diff --git a/connection_timer_test.go b/connection_timer_test.go new file mode 100644 index 00000000000..326a54d1f61 --- /dev/null +++ b/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)) + }) +}) diff --git a/internal/utils/timer.go b/internal/utils/timer.go index a4f5e67aa0e..361106c8a95 100644 --- a/internal/utils/timer.go +++ b/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() diff --git a/internal/utils/timer_test.go b/internal/utils/timer_test.go index 0cbb4a0111f..973bbae9cd4 100644 --- a/internal/utils/timer_test.go +++ b/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++ {