drain received packets when the connection is closed (#4773)

* clear receivedPackets buffer on connection close

If packets were queued up in our receivedPackets queue when we closed
the connection we would fail reuse them. Eventually the GC would have to
clean this as trash.

* move drain logic to defer in run
This commit is contained in:
Marco Munizaga
2024-12-30 22:38:57 -08:00
committed by GitHub
parent 0b9bd3c4de
commit ea6d198c3d
2 changed files with 47 additions and 0 deletions

View File

@@ -496,6 +496,22 @@ func (s *connection) run() error {
var closeErr closeError
defer func() { s.ctxCancel(closeErr.err) }()
defer func() {
// Drain queued packets that will never be processed.
for {
select {
case p, ok := <-s.receivedPackets:
if !ok {
return
}
p.buffer.Decrement()
p.buffer.MaybeRelease()
default:
return
}
}
}()
s.timer = *newTimer()
if err := s.cryptoStreamHandler.StartHandshake(s.ctx); err != nil {

View File

@@ -476,6 +476,37 @@ var _ = Describe("Connection", func() {
Expect(conn.Context().Done()).To(BeClosed())
})
It("Clears any pending receivedPackets", func() {
conn.handshakeComplete = true
runConn()
streamManager.EXPECT().CloseWithError(&qerr.ApplicationError{})
expectReplaceWithClosed()
cryptoSetup.EXPECT().Close()
packer.EXPECT().PackApplicationClose(gomock.Any(), gomock.Any(), conn.version).Return(&coalescedPacket{buffer: getPacketBuffer()}, nil)
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
gomock.InOrder(
tracer.EXPECT().ClosedConnection(gomock.Any()).Do(func(e error) {
// Send any old packet. It should get dropped.
conn.handlePacket(receivedPacket{
rcvTime: time.Now(),
remoteAddr: &net.UDPAddr{},
buffer: getPacketBuffer(),
data: []byte("foobar"),
})
var appErr *ApplicationError
Expect(errors.As(e, &appErr)).To(BeTrue())
Expect(appErr.Remote).To(BeFalse())
Expect(appErr.ErrorCode).To(BeZero())
}),
tracer.EXPECT().Close(),
)
conn.CloseWithError(0, "")
Eventually(areConnsRunning).Should(BeFalse())
Expect(conn.Context().Done()).To(BeClosed())
Expect(len(conn.receivedPackets)).To(BeZero())
})
It("only closes once", func() {
runConn()
streamManager.EXPECT().CloseWithError(gomock.Any())