From a0fb14381e78095885ebbb1d11ed3427ba97b175 Mon Sep 17 00:00:00 2001 From: Lucas Clemente Date: Thu, 8 Sep 2016 11:21:03 +0200 Subject: [PATCH] don't send more packets when there are too many unacked fixes #249, fixes #322 --- ackhandler/interfaces.go | 2 +- ackhandler/sent_packet_handler.go | 6 ++++-- ackhandler/sent_packet_handler_test.go | 12 +++++++++--- session.go | 2 +- session_test.go | 6 +++--- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index 9338906a0..ee7ac5957 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -20,7 +20,7 @@ type SentPacketHandler interface { BytesInFlight() protocol.ByteCount GetLeastUnacked() protocol.PacketNumber - CongestionAllowsSending() bool + SendingAllowed() bool CheckForError() error TimeOfFirstRTO() time.Time diff --git a/ackhandler/sent_packet_handler.go b/ackhandler/sent_packet_handler.go index 5b359dd49..236f08f15 100644 --- a/ackhandler/sent_packet_handler.go +++ b/ackhandler/sent_packet_handler.go @@ -269,8 +269,10 @@ func (h *sentPacketHandler) GetStopWaitingFrame(force bool) *frames.StopWaitingF return h.stopWaitingManager.GetStopWaitingFrame(force) } -func (h *sentPacketHandler) CongestionAllowsSending() bool { - return h.BytesInFlight() <= h.congestion.GetCongestionWindow() +func (h *sentPacketHandler) SendingAllowed() bool { + congestionLimited := h.BytesInFlight() > h.congestion.GetCongestionWindow() + maxTrackedLimited := protocol.PacketNumber(len(h.retransmissionQueue)+h.packetHistory.Len()) >= protocol.MaxTrackedSentPackets + return !(congestionLimited || maxTrackedLimited) } func (h *sentPacketHandler) CheckForError() error { diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index 48b3abe65..f507531a5 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -741,11 +741,17 @@ var _ = Describe("SentPacketHandler", func() { Expect(cong.argsOnCongestionEvent[3]).To(Equal(congestion.PacketVector{{Number: 2, Length: 2}})) }) - It("allows or denies sending", func() { - Expect(handler.CongestionAllowsSending()).To(BeTrue()) + It("allows or denies sending based on congestion", func() { + Expect(handler.SendingAllowed()).To(BeTrue()) err := handler.SentPacket(&Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: protocol.DefaultTCPMSS + 1}) Expect(err).NotTo(HaveOccurred()) - Expect(handler.CongestionAllowsSending()).To(BeFalse()) + Expect(handler.SendingAllowed()).To(BeFalse()) + }) + + It("allows or denies sending based on the number of tracked packets", func() { + Expect(handler.SendingAllowed()).To(BeTrue()) + handler.retransmissionQueue = make([]*Packet, protocol.MaxTrackedSentPackets) + Expect(handler.SendingAllowed()).To(BeFalse()) }) It("should call OnRetransmissionTimeout", func() { diff --git a/session.go b/session.go index 1ec39b49f..36ffc64ae 100644 --- a/session.go +++ b/session.go @@ -444,7 +444,7 @@ func (s *Session) sendPacket() error { // Do this before checking the congestion, since we might de-congestionize here :) s.sentPacketHandler.MaybeQueueRTOs() - if !s.sentPacketHandler.CongestionAllowsSending() { + if !s.sentPacketHandler.SendingAllowed() { return nil } diff --git a/session_test.go b/session_test.go index afba24d33..632f1a5b9 100644 --- a/session_test.go +++ b/session_test.go @@ -67,9 +67,9 @@ func (h *mockSentPacketHandler) GetStopWaitingFrame(force bool) *frames.StopWait h.requestedStopWaiting = true return &frames.StopWaitingFrame{LeastUnacked: 0x1337} } -func (h *mockSentPacketHandler) CongestionAllowsSending() bool { return !h.congestionLimited } -func (h *mockSentPacketHandler) CheckForError() error { return nil } -func (h *mockSentPacketHandler) TimeOfFirstRTO() time.Time { panic("not implemented") } +func (h *mockSentPacketHandler) SendingAllowed() bool { return !h.congestionLimited } +func (h *mockSentPacketHandler) CheckForError() error { return nil } +func (h *mockSentPacketHandler) TimeOfFirstRTO() time.Time { panic("not implemented") } func (h *mockSentPacketHandler) MaybeQueueRTOs() { h.maybeQueueRTOsCalled = true