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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ef9bd19fc9
commit
fe322ce067
@@ -16,6 +16,10 @@ import (
|
|||||||
// DefaultPort is the default port on which Mumble servers listen.
|
// DefaultPort is the default port on which Mumble servers listen.
|
||||||
const DefaultPort = 64738
|
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.
|
// Conn represents a control protocol connection to a Mumble client/server.
|
||||||
type Conn struct {
|
type Conn struct {
|
||||||
sync.Mutex
|
sync.Mutex
|
||||||
@@ -54,6 +58,11 @@ func (c *Conn) ReadPacket() (uint16, []byte, error) {
|
|||||||
}
|
}
|
||||||
if pLengthInt > len(c.buffer) {
|
if pLengthInt > len(c.buffer) {
|
||||||
c.buffer = make([]byte, pLengthInt)
|
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 {
|
if _, err := io.ReadFull(c.Conn, c.buffer[:pLengthInt]); err != nil {
|
||||||
return 0, nil, err
|
return 0, nil, err
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user