Merge pull request #2581 from lucas-clemente/fix-early-retransmit

don't switch to PTO mode after using early loss detection
quic-go · May 31, 2020 · 90acaf8 · 90acaf8
2 parents fe85b52 + 64ef72f
commit 90acaf8
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 2 deletions.
diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go
@@ -603,6 +603,7 @@ func (h *sentPacketHandler) onVerifiedLossDetectionTimeout() error {
 		for _, p := range lostPackets {
 			h.congestion.OnPacketLost(p.PacketNumber, p.Length, priorInFlight)
 		}
+		return nil
 	}
 
 	// PTO

diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go
@@ -913,11 +913,12 @@ var _ = Describe("SentPacketHandler", func() {
 		})
 
 		It("sets the early retransmit alarm", func() {
+			handler.ReceivedPacket(protocol.EncryptionHandshake)
 			handler.handshakeComplete = true
 			now := time.Now()
 			handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-2 * time.Second)}))
 			handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-2 * time.Second)}))
-			handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 3, SendTime: now.Add(-time.Second)}))
+			handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 3, SendTime: now}))
 			Expect(handler.appDataPackets.lossTime.IsZero()).To(BeTrue())
 
 			ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}}
@@ -926,13 +927,20 @@ var _ = Describe("SentPacketHandler", func() {
 
 			// Packet 1 should be considered lost (1+1/8) RTTs after it was sent.
 			Expect(handler.GetLossDetectionTimeout().Sub(getPacket(1, protocol.Encryption1RTT).SendTime)).To(Equal(time.Second * 9 / 8))
+			Expect(handler.SendMode()).To(Equal(SendAny))
+
+			expectInPacketHistory([]protocol.PacketNumber{1, 3}, protocol.Encryption1RTT)
+			Expect(handler.OnLossDetectionTimeout()).To(Succeed())
+			expectInPacketHistory([]protocol.PacketNumber{3}, protocol.Encryption1RTT)
+			Expect(handler.SendMode()).To(Equal(SendAny))
 		})
 
 		It("sets the early retransmit alarm for crypto packets", func() {
+			handler.ReceivedBytes(1000)
 			now := time.Now()
 			handler.SentPacket(initialPacket(&Packet{PacketNumber: 1, SendTime: now.Add(-2 * time.Second)}))
 			handler.SentPacket(initialPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-2 * time.Second)}))
-			handler.SentPacket(initialPacket(&Packet{PacketNumber: 3, SendTime: now.Add(-time.Second)}))
+			handler.SentPacket(initialPacket(&Packet{PacketNumber: 3, SendTime: now}))
 			Expect(handler.initialPackets.lossTime.IsZero()).To(BeTrue())
 
 			ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}}
@@ -941,6 +949,12 @@ var _ = Describe("SentPacketHandler", func() {
 
 			// Packet 1 should be considered lost (1+1/8) RTTs after it was sent.
 			Expect(handler.GetLossDetectionTimeout().Sub(getPacket(1, protocol.EncryptionInitial).SendTime)).To(Equal(time.Second * 9 / 8))
+			Expect(handler.SendMode()).To(Equal(SendAny))
+
+			expectInPacketHistory([]protocol.PacketNumber{1, 3}, protocol.EncryptionInitial)
+			Expect(handler.OnLossDetectionTimeout()).To(Succeed())
+			expectInPacketHistory([]protocol.PacketNumber{3}, protocol.EncryptionInitial)
+			Expect(handler.SendMode()).To(Equal(SendAny))
 		})
 	})