Skip to content

Commit 4440c96

Browse files
SiyetSiyet
authored
Fix reference leak when decoding msgpack Ext payloads (#1109)
Fixes #1108. `Ext_New` doesn't steal the reference to `data` (it does its own `Py_INCREF`, matching the borrow contract expected by the `Ext` constructor). Both decode branches in `mpack_decode_ext` (typed `Ext` decode, and `Any` decode without an `ext_hook`) passed a fresh reference from `PyBytes_FromStringAndSize` and never released it, leaking one bytes object (the ext payload) per decoded Ext. The `datetime` (code -1) and `ext_hook` branches were not affected. The fix releases the caller's reference after constructing the `Ext`, same pattern as the `Raw.copy()` leak fix (#709). Verified empirically: RSS delta over 200k decodes of a 1 KiB payload went from ~205 MiB to 0 on both affected branches; the new test fails before the fix (`sys.getrefcount` 3 vs 2) and passes after. Thanks @K-ANOY for the precise report, the ownership analysis there was spot on. Co-authored-by: Siyet <[email protected]>
1 parent 52ae4f1 commit 4440c96

2 files changed

Lines changed: 18 additions & 2 deletions

File tree

‎src/msgspec/_core.c‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16486,7 +16486,10 @@ mpack_decode_ext(
1648616486
else if (type->types & MS_TYPE_EXT) {
1648716487
data = PyBytes_FromStringAndSize(data_buf, size);
1648816488
if (data == NULL) return NULL;
16489-
return Ext_New(code, data);
16489+
/* Ext_New doesn't steal the reference to data */
16490+
out = Ext_New(code, data);
16491+
Py_DECREF(data);
16492+
return out;
1649016493
}
1649116494
else if (!(type->types & MS_TYPE_ANY)) {
1649216495
return ms_validation_error("ext", type, path);
@@ -16503,7 +16506,10 @@ mpack_decode_ext(
1650316506
else if (self->ext_hook == NULL) {
1650416507
data = PyBytes_FromStringAndSize(data_buf, size);
1650516508
if (data == NULL) return NULL;
16506-
return Ext_New(code, data);
16509+
/* Ext_New doesn't steal the reference to data */
16510+
out = Ext_New(code, data);
16511+
Py_DECREF(data);
16512+
return out;
1650716513
}
1650816514
else {
1650916515
pycode = PyLong_FromLong(code);

‎tests/unit/test_msgpack.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,6 +1339,16 @@ def test_roundtrip_typed_decoder(self, size):
13391339
out = dec.decode(buf)
13401340
assert out == ext
13411341

1342+
@pytest.mark.parametrize("type", [None, msgspec.msgpack.Ext])
1343+
def test_decode_ext_no_reference_leak(self, type):
1344+
"""https://github.com/msgspec/msgspec/issues/1108"""
1345+
buf = msgspec.msgpack.encode(msgspec.msgpack.Ext(5, b"ext-leak-test-payload"))
1346+
kwargs = {} if type is None else {"type": type}
1347+
ext = msgspec.msgpack.decode(buf, **kwargs)
1348+
data = ext.data
1349+
del ext
1350+
assert sys.getrefcount(data) <= 2 # `data` + getrefcount argument
1351+
13421352
def test_typed_decoder_skips_ext_hook(self):
13431353
def ext_hook(code, data):
13441354
assert False, "shouldn't ever get called"

0 commit comments

Comments
 (0)