Repository: quic-go/quic-go

Issue:
quic-go 0.50.0 crashes
Since I upgraded to the latest version, I see it crashing regularly with the following callstack:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x63 pc=0x785dd2]

goroutine 139654190 [running]:
github.com/quic-go/quic-go/internal/ackhandler.(*packet).outstanding(...)
        /root/go/pkg/mod/github.com/quic-go/quic-go@v0.50.0/internal/ackhandler/packet.go:29
github.com/quic-go/quic-go/internal/ackhandler.(*sentPacketHistory).Remove(0xc0598815f0?, 0xc6a080?)
        /root/go/pkg/mod/github.com/quic-go/quic-go@v0.50.0/internal/ackhandler/sent_packet_history.go:126 +0x72
github.com/quic-go/quic-go/internal/ackhandler.(*sentPacketHandler).detectLostPathProbes(0xc0263f9880, {0xc000d33dae?, 0x4?, 0x10b36a0?})
        /root/go/pkg/mod/github.com/quic-go/quic-go@v0.50.0/internal/ackhandler/sent_packet_handler.go:661 +0x194
github.com/quic-go/quic-go/internal/ackhandler.(*sentPacketHandler).OnLossDetectionTimeout(0xc0263f9880, {0xf7f1a2e65474?, 0x10b36a0?, 0x10b36a0?})
        /root/go/pkg/mod/github.com/quic-go/quic-go@v0.50.0/internal/ackhandler/sent_packet_handler.go:736 +0xa5
github.com/quic-go/quic-go.(*connection).run(0xc011367808)
        /root/go/pkg/mod/github.com/quic-go/quic-go@v0.50.0/connection.go:621 +0x678
created by github.com/quic-go/quic-go.(*baseServer).handleInitialImpl in goroutine 42
        /root/go/pkg/mod/github.com/quic-go/quic-go@v0.50.0/server.go:745 +0xd4d

Discussion:
@marten-seemann: Can you provide some more details what you're doing when this happens?

Solution patch:
diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go
index 7c3cf892..83d2736a 100644
--- a/internal/ackhandler/sent_packet_handler.go
+++ b/internal/ackhandler/sent_packet_handler.go
@@ -460,10 +460,10 @@ func (h *sentPacketHandler) detectAndRemoveAckedPackets(ack *wire.AckFrame, encL
 		}
 		if p.isPathProbePacket {
 			probePacket := pnSpace.history.RemovePathProbe(p.PacketNumber)
-			if probePacket == nil {
-				panic(fmt.Sprintf("path probe doesn't exist: %d", p.PacketNumber))
+			// the probe packet might already have been declared lost
+			if probePacket != nil {
+				h.ackedPackets = append(h.ackedPackets, probePacket)
 			}
-			h.ackedPackets = append(h.ackedPackets, probePacket)
 			continue
 		}
 		h.ackedPackets = append(h.ackedPackets, p)
@@ -658,7 +658,6 @@ func (h *sentPacketHandler) detectLostPathProbes(now time.Time) {
 		for _, f := range p.Frames {
 			f.Handler.OnLost(f.Frame)
 		}
-		h.appDataPackets.history.Remove(p.PacketNumber)
 		h.appDataPackets.history.RemovePathProbe(p.PacketNumber)
 	}
 }



Test patch:
diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go
index de1335ed..54d5e633 100644
--- a/internal/ackhandler/sent_packet_handler_test.go
+++ b/internal/ackhandler/sent_packet_handler_test.go
@@ -1,7 +1,9 @@
 package ackhandler
 
 import (
+	"encoding/binary"
 	"fmt"
+	"math/rand/v2"
 	"slices"
 	"testing"
 	"time"
@@ -39,6 +41,11 @@ type packetTracker struct {
 	Lost  []protocol.PacketNumber
 }
 
+func (t *packetTracker) Reset() {
+	t.Acked = nil
+	t.Lost = nil
+}
+
 func (t *packetTracker) NewPingFrame(pn protocol.PacketNumber) Frame {
 	return Frame{
 		Frame: &wire.PingFrame{},
@@ -1227,3 +1234,91 @@ func TestSentPacketHandlerPathProbeAckAndLoss(t *testing.T) {
 
 	require.Equal(t, t2.Add(pathProbePacketLossTimeout), sph.GetLossDetectionTimeout())
 }
+
+// The packet tracking logic is pretty complex.
+// We test it with a randomized approach, to make sure that it doesn't panic under any circumstances.
+func TestSentPacketHandlerRandomized(t *testing.T) {
+	seed := uint64(time.Now().UnixNano())
+	for i := range 5 {
+		t.Run(fmt.Sprintf("run %d (seed %d)", i+1, seed), func(t *testing.T) {
+			testSentPacketHandlerRandomized(t, seed)
+		})
+		seed++
+	}
+}
+
+func testSentPacketHandlerRandomized(t *testing.T, seed uint64) {
+	var b [32]byte
+	binary.BigEndian.PutUint64(b[:], seed)
+	r := rand.New(rand.NewChaCha8(b))
+
+	var rttStats utils.RTTStats
+	rtt := []time.Duration{10 * time.Millisecond, 100 * time.Millisecond, 1000 * time.Millisecond}[r.IntN(3)]
+	t.Logf("rtt: %dms", rtt.Milliseconds())
+	rttStats.UpdateRTT(rtt, 0) // RTT of the original path
+
+	randDuration := func(min, max time.Duration) time.Duration {
+		return time.Duration(rand.Int64N(int64(max-min))) + min
+	}
+
+	sph := newSentPacketHandler(
+		0,
+		1200,
+		&rttStats,
+		true,
+		false,
+		protocol.PerspectiveClient,
+		nil,
+		utils.DefaultLogger,
+	)
+	sph.DropPackets(protocol.EncryptionInitial, time.Now())
+	sph.DropPackets(protocol.EncryptionHandshake, time.Now())
+
+	var packets packetTracker
+	sendPacket := func(ti time.Time, isPathProbe bool) protocol.PacketNumber {
+		pn := sph.PopPacketNumber(protocol.Encryption1RTT)
+		sph.SentPacket(ti, pn, protocol.InvalidPacketNumber, nil, []Frame{packets.NewPingFrame(pn)}, protocol.Encryption1RTT, protocol.ECNNon, 1200, false, isPathProbe)
+		return pn
+	}
+
+	now := time.Now()
+	start := now
+	var pns []protocol.PacketNumber
+	for range 4 {
+		isProbe := r.Int()%2 == 0
+		pn := sendPacket(now, isProbe)
+		t.Logf("t=%dms: sending packet %d (probe packet: %t)", now.Sub(start).Milliseconds(), pn, isProbe)
+		pns = append(pns, pn)
+		now = now.Add(randDuration(0, 500*time.Millisecond))
+		if r.Int()%3 == 0 {
+			sph.OnLossDetectionTimeout(now)
+			t.Logf("t=%dms: loss detection timeout (lost: %v)", now.Sub(start).Milliseconds(), packets.Lost)
+			packets.Reset()
+			now = now.Add(randDuration(0, 500*time.Millisecond))
+		}
+		if r.Int()%3 == 0 {
+			// acknowledge up to 2 random packet numbers from the pns slice
+			var ackPns []protocol.PacketNumber
+			if len(pns) > 0 {
+				numToAck := min(1+r.IntN(2), len(pns))
+				for range numToAck {
+					ackPns = append(ackPns, pns[r.IntN(len(pns))])
+				}
+			}
+			if len(ackPns) > 1 {
+				slices.Sort(ackPns)
+				ackPns = slices.Compact(ackPns)
+			}
+			sph.ReceivedAck(&wire.AckFrame{AckRanges: ackRanges(ackPns...)}, protocol.Encryption1RTT, now)
+			t.Logf("t=%dms: received ACK for packets %v (acked: %v, lost: %v)", now.Sub(start).Milliseconds(), ackPns, packets.Acked, packets.Lost)
+			packets.Reset()
+			now = now.Add(randDuration(0, 500*time.Millisecond))
+		}
+		if r.Int()%10 == 0 {
+			sph.MigratedPath(now, 1200)
+			now = now.Add(randDuration(0, 500*time.Millisecond))
+		}
+	}
+	t.Logf("t=%dms: loss detection timeout (lost: %v)", now.Sub(start).Milliseconds(), packets.Lost)
+	sph.OnLossDetectionTimeout(now)
+}
diff --git a/server_test.go b/server_test.go
index dbaef946..fac0b24a 100644
--- a/server_test.go
+++ b/server_test.go
@@ -786,9 +786,9 @@ func TestServerClose(t *testing.T) {
 func TestServerGetConfigForClientAccept(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	c := NewMockQUICConn(mockCtrl)
-	c.EXPECT().run()
-	c.EXPECT().Context().Return(context.Background())
-	c.EXPECT().HandshakeComplete().Return(make(chan struct{}))
+	c.EXPECT().run().MaxTimes(1)
+	c.EXPECT().Context().Return(context.Background()).MaxTimes(1)
+	c.EXPECT().HandshakeComplete().Return(make(chan struct{})).MaxTimes(1)
 	recorder := newConnConstructorRecorder(c)
 	server := newTestServer(t, &serverOpts{
 		config: &Config{
@@ -805,7 +805,7 @@ func TestServerGetConfigForClientAccept(t *testing.T) {
 		protocol.ParseConnectionID([]byte{5, 4, 3, 2, 1}),
 		protocol.ParseConnectionID([]byte{1, 2, 3, 4, 5, 6, 7, 8}),
 	)
-	c.EXPECT().handlePacket(packet)
+	c.EXPECT().handlePacket(packet).MaxTimes(1)
 
 	server.handlePacket(packet)