From af3f69c0f1197ac7ffa60f3718d8f78fd96396d1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 2 Aug 2016 13:38:13 +0700 Subject: [PATCH] always delete packets from packetHistory when receiving a StopWaiting in new AckHandler fixes #239 --- ackhandler/received_packet_handler.go | 9 +++------ ackhandler/received_packet_handler_test.go | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/ackhandler/received_packet_handler.go b/ackhandler/received_packet_handler.go index 3ca1b8d1d..fd3e56950 100644 --- a/ackhandler/received_packet_handler.go +++ b/ackhandler/received_packet_handler.go @@ -89,13 +89,10 @@ func (h *receivedPacketHandler) ReceivedStopWaiting(f *frames.StopWaitingFrame) h.ignorePacketsBelow = f.LeastUnacked - 1 h.garbageCollectReceivedTimes() - // ignore if StopWaiting is unneeded, since all packets below have already been received - if h.largestInOrderObserved >= f.LeastUnacked { - return nil - } - // the LeastUnacked is the smallest packet number of any packet for which the sender is still awaiting an ack. So the largestInOrderObserved is one less than that - h.largestInOrderObserved = f.LeastUnacked - 1 + if f.LeastUnacked > h.largestInOrderObserved { + h.largestInOrderObserved = f.LeastUnacked - 1 + } h.packetHistory.DeleteBelow(f.LeastUnacked) diff --git a/ackhandler/received_packet_handler_test.go b/ackhandler/received_packet_handler_test.go index a84e5a254..956e4b718 100644 --- a/ackhandler/received_packet_handler_test.go +++ b/ackhandler/received_packet_handler_test.go @@ -229,7 +229,7 @@ var _ = Describe("receivedPacketHandler", func() { Expect(ack.AckRanges).To(BeEmpty()) }) - It("does send old ACK ranges after receiving a StopWaiting", func() { + It("doesn't send old ACK ranges after receiving a StopWaiting", func() { err := handler.ReceivedPacket(5, false) Expect(err).ToNot(HaveOccurred()) err = handler.ReceivedPacket(10, false) @@ -240,12 +240,30 @@ var _ = Describe("receivedPacketHandler", func() { Expect(err).ToNot(HaveOccurred()) err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(11)}) Expect(err).ToNot(HaveOccurred()) - ack, _ := handler.GetAckFrame(true) + ack, err := handler.GetAckFrame(true) + Expect(err).ToNot(HaveOccurred()) Expect(ack).ToNot(BeNil()) Expect(ack.LargestAcked).To(Equal(protocol.PacketNumber(12))) Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(11))) Expect(ack.HasMissingRanges()).To(BeFalse()) }) + + It("deletes packets from the packetHistory after receiving a StopWaiting, after continously received packets", func() { + for i := 1; i <= 12; i++ { + err := handler.ReceivedPacket(protocol.PacketNumber(i), false) + Expect(err).ToNot(HaveOccurred()) + } + err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)}) + Expect(err).ToNot(HaveOccurred()) + Expect(handler.largestInOrderObserved).To(Equal(protocol.PacketNumber(12))) + // check that the packets were deleted from the receivedPacketHistory by checking the values in an ACK frame + ack, err := handler.GetAckFrame(true) + Expect(err).ToNot(HaveOccurred()) + Expect(ack).ToNot(BeNil()) + Expect(ack.LargestAcked).To(Equal(protocol.PacketNumber(12))) + Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(6))) + Expect(ack.HasMissingRanges()).To(BeFalse()) + }) }) Context("Garbage Collector", func() {