From fe322ce06716f3f221d282f948a4b962bf36dd08 Mon Sep 17 00:00:00 2001 From: Brandon McGinty Date: Mon, 24 Aug 2026 12:32:44 -0400 Subject: [PATCH] release the read buffer after an oversized packet The buffer grew to the largest packet ever received and was never given back, so a single large ACL, user list or channel comment pinned its full size, up to the ten megabyte packet limit, for the life of the connection. Keep a modest buffer between packets and allocate larger ones only for as long as they are needed. Co-Authored-By: Claude Opus 5 --- gumble/gumble/conn.go | 9 ++++ gumble/gumble/conn_buffer_regression_test.go | 43 ++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 gumble/gumble/conn_buffer_regression_test.go diff --git a/gumble/gumble/conn.go b/gumble/gumble/conn.go index ed58cfa..6703958 100644 --- a/gumble/gumble/conn.go +++ b/gumble/gumble/conn.go @@ -16,6 +16,10 @@ import ( // DefaultPort is the default port on which Mumble servers listen. const DefaultPort = 64738 +// retainedPacketBytes is the largest read buffer kept between packets. Bigger +// buffers are allocated as needed and released again afterwards. +const retainedPacketBytes = 64 * 1024 + // Conn represents a control protocol connection to a Mumble client/server. type Conn struct { sync.Mutex @@ -54,6 +58,11 @@ func (c *Conn) ReadPacket() (uint16, []byte, error) { } if pLengthInt > len(c.buffer) { c.buffer = make([]byte, pLengthInt) + } else if len(c.buffer) > retainedPacketBytes && pLengthInt <= retainedPacketBytes { + // One oversized packet — a large ACL, user list or channel comment — + // used to pin its full size for the life of the connection. Give the + // memory back once ordinary traffic resumes. + c.buffer = make([]byte, retainedPacketBytes) } if _, err := io.ReadFull(c.Conn, c.buffer[:pLengthInt]); err != nil { return 0, nil, err diff --git a/gumble/gumble/conn_buffer_regression_test.go b/gumble/gumble/conn_buffer_regression_test.go new file mode 100644 index 0000000..5b35aa7 --- /dev/null +++ b/gumble/gumble/conn_buffer_regression_test.go @@ -0,0 +1,43 @@ +package gumble + +import ( + "encoding/binary" + "net" + "testing" +) + +// Regression: the read buffer grew to the largest packet ever seen and kept +// that memory for the life of the connection. +func TestConnBufferShrinksAfterAnOversizedPacket(t *testing.T) { + server, client := net.Pipe() + defer server.Close() + conn := NewConn(client) + + big := 4 * 1024 * 1024 + go func() { + defer server.Close() + writePacket := func(length int) { + var header [6]byte + binary.BigEndian.PutUint16(header[:], 3) + binary.BigEndian.PutUint32(header[2:], uint32(length)) + server.Write(header[:]) + server.Write(make([]byte, length)) + } + writePacket(big) + writePacket(128) + }() + + if _, _, err := conn.ReadPacket(); err != nil { + t.Fatalf("reading the oversized packet: %v", err) + } + if len(conn.buffer) < big { + t.Fatalf("oversized packet should have grown the buffer, got %d", len(conn.buffer)) + } + if _, _, err := conn.ReadPacket(); err != nil { + t.Fatalf("reading the small packet: %v", err) + } + if len(conn.buffer) > retainedPacketBytes { + t.Fatalf("buffer stayed at %d bytes after a small packet, above the %d retained size", + len(conn.buffer), retainedPacketBytes) + } +}