don't send the same StopWaitingFrame twice

fixes #229

ackhandler/sent_packet_handler.go

@@ -35,7 +35,8 @@ type sentPacketHandler struct {
 	largestReceivedPacketWithAck protocol.PacketNumber
 
 	// TODO: Move into separate class as in chromium
-	packetHistory map[protocol.PacketNumber]*ackhandlerlegacy.Packet
+	packetHistory      map[protocol.PacketNumber]*ackhandlerlegacy.Packet
+	stopWaitingManager stopWaitingManager
 
 	retransmissionQueue []*ackhandlerlegacy.Packet
 
@@ -58,9 +59,10 @@ func NewSentPacketHandler() SentPacketHandler {
 	)
 
 	return &sentPacketHandler{
-		packetHistory: make(map[protocol.PacketNumber]*ackhandlerlegacy.Packet),
-		rttStats:      rttStats,
-		congestion:    congestion,
+		packetHistory:      make(map[protocol.PacketNumber]*ackhandlerlegacy.Packet),
+		stopWaitingManager: stopWaitingManager{},
+		rttStats:           rttStats,
+		congestion:         congestion,
 	}
 }
 
@@ -220,6 +222,8 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *frames.AckFrame, withPacketNum
 		}
 	}
 
+	h.stopWaitingManager.ReceivedAck(ackFrame)
+
 	h.congestion.OnCongestionEvent(
 		true, /* TODO: rtt updated */
 		h.BytesInFlight(),
@@ -273,13 +277,7 @@ func (h *sentPacketHandler) GetLargestAcked() protocol.PacketNumber {
 }
 
 func (h *sentPacketHandler) GetStopWaitingFrame() *frames.StopWaitingFrame {
-	if h.LargestAcked == 0 {
-		return nil
-	}
-
-	return &frames.StopWaitingFrame{
-		LeastUnacked: h.LargestAcked + 1,
-	}
+	return h.stopWaitingManager.GetStopWaitingFrame()
 }
 
 func (h *sentPacketHandler) CongestionAllowsSending() bool {

ackhandler/sent_packet_handler_test.go

@@ -351,13 +351,11 @@ var _ = Describe("SentPacketHandler", func() {
 		})
 
 		Context("StopWaitings", func() {
-			It("does not get a StopWaiting if no ACKs haven't been received yet", func() {
-				Expect(handler.GetStopWaitingFrame()).To(BeNil())
-			})
-
 			It("gets a StopWaitingFrame", func() {
-				handler.LargestAcked = 1336
-				Expect(handler.GetStopWaitingFrame()).To(Equal(&frames.StopWaitingFrame{LeastUnacked: 1337}))
+				ack := frames.AckFrame{LargestAcked: 5, LowestAcked: 5}
+				err := handler.ReceivedAck(&ack, 1)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(handler.GetStopWaitingFrame()).To(Equal(&frames.StopWaitingFrame{LeastUnacked: 6}))
 			})
 		})
 

ackhandler/stop_waiting_manager.go

@@ -0,0 +1,29 @@
+package ackhandler
+
+import (
+	"github.com/lucas-clemente/quic-go/frames"
+	"github.com/lucas-clemente/quic-go/protocol"
+)
+
+// This stopWaitingManager is not supposed to satisfy the StopWaitingManager interface, which is a remnant of the legacy AckHandler, and should be remove once we drop support for QUIC 33
+type stopWaitingManager struct {
+	largestLeastUnackedSent protocol.PacketNumber
+	nextLeastUnacked        protocol.PacketNumber
+}
+
+func (s *stopWaitingManager) GetStopWaitingFrame() *frames.StopWaitingFrame {
+	if s.nextLeastUnacked <= s.largestLeastUnackedSent {
+		return nil
+	}
+
+	s.largestLeastUnackedSent = s.nextLeastUnacked
+	return &frames.StopWaitingFrame{
+		LeastUnacked: s.nextLeastUnacked,
+	}
+}
+
+func (s *stopWaitingManager) ReceivedAck(ack *frames.AckFrame) {
+	if ack.LargestAcked >= s.nextLeastUnacked {
+		s.nextLeastUnacked = ack.LargestAcked + 1
+	}
+}

ackhandler/stop_waiting_manager_test.go

@@ -0,0 +1,35 @@
+package ackhandler
+
+import (
+	"github.com/lucas-clemente/quic-go/frames"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("StopWaitingManager", func() {
+	var manager *stopWaitingManager
+	BeforeEach(func() {
+		manager = &stopWaitingManager{}
+	})
+
+	It("returns nil in the beginning", func() {
+		Expect(manager.GetStopWaitingFrame()).To(BeNil())
+	})
+
+	It("returns a StopWaitingFrame, when a new ACK arrives", func() {
+		manager.ReceivedAck(&frames.AckFrame{LargestAcked: 10})
+		Expect(manager.GetStopWaitingFrame()).To(Equal(&frames.StopWaitingFrame{LeastUnacked: 11}))
+	})
+
+	It("does not decrease the LeastUnacked", func() {
+		manager.ReceivedAck(&frames.AckFrame{LargestAcked: 10})
+		manager.ReceivedAck(&frames.AckFrame{LargestAcked: 9})
+		Expect(manager.GetStopWaitingFrame()).To(Equal(&frames.StopWaitingFrame{LeastUnacked: 11}))
+	})
+
+	It("does not send the same StopWaitingFrame twice", func() {
+		manager.ReceivedAck(&frames.AckFrame{LargestAcked: 10})
+		Expect(manager.GetStopWaitingFrame()).ToNot(BeNil())
+		Expect(manager.GetStopWaitingFrame()).To(BeNil())
+	})
+})