Skip to content

Commit

Permalink
Fix memory leak in Raw.copy() (#709)
Browse files Browse the repository at this point in the history
* Fix memory leak in Raw.copy().

Raw_New borrows a reference to its PyBytesObject argument, so Raw_Copy must
still dispose of it.

The following is a minimal repro for this issue:

import msgspec

DECODER = msgspec.msgpack.Decoder(type=msgspec.Raw)
PAYLOAD = b"\xda\xff\xff" + (65535 * b"\0") # array of 65535 zeros

for _ in range(1000000):
  raw = DECODER.decode(PAYLOAD)
  raw.copy()

* Add test for raw copy memleak

---------

Co-authored-by: Jim Crist-Harif <jcristharif@gmail.com>
  • Loading branch information
tjgq and jcrist authored Oct 13, 2024
1 parent 8963db3 commit 51a261c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
4 changes: 3 additions & 1 deletion msgspec/_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,9 @@ Raw_copy(Raw *self, PyObject *unused)
}
PyObject *buf = PyBytes_FromStringAndSize(self->buf, self->len);
if (buf == NULL) return NULL;
return Raw_New(buf);
PyObject *out = Raw_New(buf);
Py_DECREF(buf);
return out;
}

static PyMethodDef Raw_methods[] = {
Expand Down
25 changes: 25 additions & 0 deletions tests/test_raw.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import operator
import subprocess
import sys
import textwrap
import weakref

import pytest
Expand Down Expand Up @@ -69,6 +71,29 @@ def test_raw_copy():
assert ref() is None


def test_raw_copy_doesnt_leak():
"""See https://github.com/jcrist/msgspec/pull/709"""
script = textwrap.dedent(
"""
import msgspec
import tracemalloc
tracemalloc.start()
raw = msgspec.Raw(bytearray(1000))
for _ in range(10000):
raw.copy()
_, peak = tracemalloc.get_traced_memory()
print(peak)
"""
)

output = subprocess.check_output([sys.executable, "-c", script])
peak = int(output.decode().strip())
assert peak < 10_000 # should really be ~2000


def test_raw_pickle_bytes():
orig_buffer = b"test"
r = msgspec.Raw(orig_buffer)
Expand Down

0 comments on commit 51a261c

Please sign in to comment.