From 73108c2a511f522ee2ff4b1fdf2bc2bb0bb687a7 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Tue, 28 Jan 2025 15:10:18 +0100
Subject: [PATCH] fix connection busy-looping when pacing with a blocked send
 queue (#4943)

---
 connection.go      |  8 +++++--
 connection_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/connection.go b/connection.go
index df0ca314..b5287938 100644
--- a/connection.go
+++ b/connection.go
@@ -637,9 +637,10 @@ runLoop:
 		}
 
 		if s.sendQueue.WouldBlock() {
-			// The send queue is still busy sending out packets.
-			// Wait until there's space to enqueue new packets.
+			// The send queue is still busy sending out packets. Wait until there's space to enqueue new packets.
 			sendQueueAvailable = s.sendQueue.Available()
+			// Cancel the pacing timer, as we can't send any more packets until the send queue is available again.
+			s.pacingDeadline = time.Time{}
 			continue
 		}
 
@@ -652,7 +653,10 @@ runLoop:
 			break runLoop
 		}
 		if s.sendQueue.WouldBlock() {
+			// The send queue is still busy sending out packets. Wait until there's space to enqueue new packets.
 			sendQueueAvailable = s.sendQueue.Available()
+			// Cancel the pacing timer, as we can't send any more packets until the send queue is available again.
+			s.pacingDeadline = time.Time{}
 		} else {
 			sendQueueAvailable = nil
 		}
diff --git a/connection_test.go b/connection_test.go
index 57f49a03..58bb51ed 100644
--- a/connection_test.go
+++ b/connection_test.go
@@ -1724,6 +1724,61 @@ func TestConnectionPacketPacing(t *testing.T) {
 	}
 }
 
+// When the send queue blocks, we need to reset the pacing timer, otherwise the run loop might busy-loop.
+// See https://github.com/quic-go/quic-go/pull/4943 for more details.
+func TestConnectionPacingAndSendQueue(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	sph := mockackhandler.NewMockSentPacketHandler(mockCtrl)
+	sender := NewMockSender(mockCtrl)
+
+	tc := newServerTestConnection(t,
+		mockCtrl,
+		nil,
+		false,
+		connectionOptSentPacketHandler(sph),
+		connectionOptSender(sender),
+		connectionOptHandshakeConfirmed(),
+		// set a fixed RTT, so that the idle timeout doesn't interfere with this test
+		connectionOptRTT(10*time.Second),
+	)
+	sender.EXPECT().Run()
+
+	sendQueueAvailable := make(chan struct{})
+	pacingDeadline := time.Now().Add(-time.Millisecond)
+	var counter int
+	// allow exactly one packet to be sent, then become blocked
+	sender.EXPECT().WouldBlock().Return(false)
+	sender.EXPECT().WouldBlock().DoAndReturn(func() bool { counter++; return true }).AnyTimes()
+	sender.EXPECT().Available().Return(sendQueueAvailable).AnyTimes()
+	sph.EXPECT().GetLossDetectionTimeout().Return(time.Now().Add(time.Hour)).AnyTimes()
+	sph.EXPECT().SendMode(gomock.Any()).Return(ackhandler.SendPacingLimited).AnyTimes()
+	sph.EXPECT().TimeUntilSend().Return(pacingDeadline).AnyTimes()
+	sph.EXPECT().ECNMode(gomock.Any()).Return(protocol.ECNNon).AnyTimes()
+	tc.packer.EXPECT().PackAckOnlyPacket(gomock.Any(), gomock.Any(), gomock.Any()).Return(
+		shortHeaderPacket{}, nil, errNothingToPack,
+	)
+
+	errChan := make(chan error, 1)
+	go func() { errChan <- tc.conn.run() }()
+	tc.conn.scheduleSending()
+
+	time.Sleep(scaleDuration(10 * time.Millisecond))
+
+	// test teardown
+	tc.connRunner.EXPECT().Remove(gomock.Any()).AnyTimes()
+	sender.EXPECT().Close()
+	tc.conn.destroy(nil)
+	select {
+	case err := <-errChan:
+		require.NoError(t, err)
+	case <-time.After(time.Second):
+		t.Fatal("timeout")
+	}
+
+	// make sure the run loop didn't do too many iterations
+	require.Less(t, counter, 3)
+}
+
 func TestConnectionIdleTimeout(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	sph := mockackhandler.NewMockSentPacketHandler(mockCtrl)