Skip to content

Commit 3a568a1

Browse files
committed
[mypyc] Fix memory leak in Exception and dict subclasses with instance attributes
Fixes #21716. On Python <3.12, when mypyc compiles a subclass of a builtin type (`Exception` or `dict`) that has instance attributes, it appends a `__dict__` slot onto the end of the base struct at `tp_dictoffset = sizeof(builtin_base)` to hold them. But no custom `tp_dealloc`/`tp_clear`/`tp_traverse` is emitted for this case, so the type inherits the base's slots — which only clear the base's own fields and know nothing about the extra slot mypyc added. The dict (and every object it references) leaks on every instance destruction. Hit downstream in sqlglot[c] (tobymao/sqlglot#7853): every raised-and-caught `ParseError` leaked ~1 KB. On 3.12+ mypyc already skips adding the extra offset for these bases, so only <3.12 is affected. Changes in `mypyc/codegen/emitclass.py`: * Extend the gate that emits `tp_dealloc`/`tp_clear`/`tp_traverse` to fire for any `builtin_base` subclass with `has_dict` under `capi < 3.12`. The existing builtin-base dealloc dance (untrack → subtype clear → re-track → base dealloc) then does the right thing: our clear frees the extra slot, then the base's dealloc frees its own state and the object. * Set `Py_TPFLAGS_HAVE_GC` explicitly. `PyType_Ready` only inherits `HAVE_GC` from the base when it also inherits both `tp_traverse` and `tp_dealloc`; overriding either drops the inheritance and without it the type's inherited `tp_new` would allocate without a GC header. * Fix a latent offset bug in `generate_traverse_for_class` / `generate_clear_for_class`: the code that visits/clears the extra dict slot used `sizeof(struct_name)`, but for `builtin_base` classes `tp_dictoffset` is `sizeof(builtin_base)`. This mismatch was previously unreachable because the emission gate never fired for `builtin_base` classes. Regression test in `mypyc/test-data/run-classes.test` uses a `Payload` class with a `__del__` counter: without the fix, 1000 fresh exceptions each keep their fresh payload alive; with it, all payloads are freed.
1 parent d5daed2 commit 3a568a1

2 files changed

Lines changed: 56 additions & 29 deletions

File tree

mypyc/codegen/emitclass.py

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,16 @@ def generate_class(cl: ClassIR, module: str, emitter: Emitter) -> None:
267267
fields["tp_new"] = new_name
268268

269269
managed_dict = has_managed_dict(cl, emitter)
270-
if generate_full or managed_dict:
270+
# On Python <3.12, subclasses of builtin types (Exception, dict) get an extra __dict__
271+
# slot that the inherited base dealloc knows nothing about. Emit our own so it gets freed.
272+
needs_builtin_dict_cleanup = (
273+
cl.builtin_base is not None
274+
and cl.has_dict
275+
and not managed_dict
276+
and emitter.capi_version < (3, 12)
277+
)
278+
generate_dealloc_slots = generate_full or managed_dict or needs_builtin_dict_cleanup
279+
if generate_dealloc_slots:
271280
fields["tp_dealloc"] = f"(destructor){name_prefix}_dealloc"
272281
if not cl.is_acyclic:
273282
fields["tp_traverse"] = f"(traverseproc){name_prefix}_traverse"
@@ -340,7 +349,7 @@ def emit_line() -> None:
340349
else:
341350
fields["tp_basicsize"] = base_size
342351

343-
if generate_full or managed_dict:
352+
if generate_dealloc_slots:
344353
if not cl.is_acyclic:
345354
generate_traverse_for_class(cl, traverse_name, emitter)
346355
emit_line()
@@ -386,7 +395,8 @@ def emit_line() -> None:
386395
emit_line()
387396

388397
flags = ["Py_TPFLAGS_DEFAULT", "Py_TPFLAGS_HEAPTYPE", "Py_TPFLAGS_BASETYPE"]
389-
if (generate_full or managed_dict) and not cl.is_acyclic:
398+
if generate_dealloc_slots and not cl.is_acyclic:
399+
# Set explicitly: PyType_Ready won't inherit HAVE_GC once we override tp_dealloc/traverse.
390400
flags.append("Py_TPFLAGS_HAVE_GC")
391401
if cl.has_method("__call__"):
392402
fields["tp_vectorcall_offset"] = "offsetof({}, vectorcall)".format(
@@ -882,14 +892,11 @@ def generate_traverse_for_class(cl: ClassIR, func_name: str, emitter: Emitter) -
882892
emitter.emit_line(f"rv = PyObject_VisitManagedDict({base_args});")
883893
emitter.emit_line("if (rv != 0) return rv;")
884894
elif cl.has_dict:
885-
struct_name = cl.struct_name(emitter.names)
886-
# __dict__ lives right after the struct and __weakref__ lives right after that
887-
emitter.emit_gc_visit(
888-
f"*((PyObject **)((char *)self + sizeof({struct_name})))", object_rprimitive
889-
)
895+
# __dict__ lives at tp_dictoffset (== base_size), __weakref__ right after it.
896+
base_size = f"sizeof({cl.builtin_base or cl.struct_name(emitter.names)})"
897+
emitter.emit_gc_visit(f"*((PyObject **)((char *)self + {base_size}))", object_rprimitive)
890898
emitter.emit_gc_visit(
891-
f"*((PyObject **)((char *)self + sizeof(PyObject *) + sizeof({struct_name})))",
892-
object_rprimitive,
899+
f"*((PyObject **)((char *)self + sizeof(PyObject *) + {base_size}))", object_rprimitive
893900
)
894901
emitter.emit_line("return rv;")
895902
emitter.emit_line("}")
@@ -908,14 +915,11 @@ def generate_clear_for_class(cl: ClassIR, func_name: str, emitter: Emitter) -> N
908915
if has_managed_dict(cl, emitter):
909916
emitter.emit_line(f"PyObject_ClearManagedDict({base_args});")
910917
elif cl.has_dict:
911-
struct_name = cl.struct_name(emitter.names)
912-
# __dict__ lives right after the struct and __weakref__ lives right after that
913-
emitter.emit_gc_clear(
914-
f"*((PyObject **)((char *)self + sizeof({struct_name})))", object_rprimitive
915-
)
918+
# __dict__ lives at tp_dictoffset (== base_size), __weakref__ right after it.
919+
base_size = f"sizeof({cl.builtin_base or cl.struct_name(emitter.names)})"
920+
emitter.emit_gc_clear(f"*((PyObject **)((char *)self + {base_size}))", object_rprimitive)
916921
emitter.emit_gc_clear(
917-
f"*((PyObject **)((char *)self + sizeof(PyObject *) + sizeof({struct_name})))",
918-
object_rprimitive,
922+
f"*((PyObject **)((char *)self + sizeof(PyObject *) + {base_size}))", object_rprimitive
919923
)
920924
emitter.emit_line("return 0;")
921925
emitter.emit_line("}")

mypyc/test-data/run-classes.test

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,40 @@ except Failure as e:
956956
assert e.x == 10
957957
heyo()
958958

959+
[case testSubclassExceptionNoAttributeLeak]
960+
# Regression test: on Python <3.12, a mypyc-compiled Exception subclass with
961+
# instance attributes used to leak its __dict__ contents on every destruction
962+
# because BaseException's inherited dealloc/clear doesn't know about the extra
963+
# __dict__ slot mypyc appends at tp_dictoffset = sizeof(PyBaseExceptionObject).
964+
class LeakyErr(Exception):
965+
def __init__(self, payload: object) -> None:
966+
self.data = payload
967+
968+
[file driver.py]
969+
import gc
970+
from native import LeakyErr
971+
972+
class Payload:
973+
live = 0
974+
def __init__(self) -> None:
975+
Payload.live += 1
976+
def __del__(self) -> None:
977+
Payload.live -= 1
978+
979+
# Each iteration creates a fresh Payload, stashes it on a fresh LeakyErr, and
980+
# drops both. If the exception's __dict__ leaks, the Payload it references
981+
# survives — __del__ never runs and Payload.live grows without bound.
982+
for _ in range(1000):
983+
LeakyErr(Payload())
984+
gc.collect()
985+
assert Payload.live == 0, f"leaked {Payload.live} payloads"
986+
987+
# Sanity-check the normal raise/catch path still works and attrs are readable.
988+
try:
989+
raise LeakyErr("hi")
990+
except LeakyErr as e:
991+
assert e.data == "hi"
992+
959993
[case testSubclassDict]
960994
from typing import Dict
961995
class WelpDict(Dict[str, int]):
@@ -3452,22 +3486,11 @@ def test_dict_subclass_dealloc() -> None:
34523486
del d
34533487

34543488
[file driver.py]
3455-
import sys
3456-
34573489
from native import events, test_dict_subclass_dealloc
34583490

34593491
test_dict_subclass_dealloc()
34603492

3461-
expected_events: list[str] = []
3462-
3463-
# TODO: Fix when compiling for older python.
3464-
# The user-defined __del__ method is currently only invoked when __dict__ is a managed dict
3465-
# because calling __del__ in tp_clear on older python crashes.
3466-
if sys.version_info >= (3, 12):
3467-
expected_events.append("deleting DictSubclass")
3468-
expected_events.append("deleting Item")
3469-
3470-
assert events == expected_events, events
3493+
assert events == ["deleting DictSubclass", "deleting Item"], events
34713494

34723495
[case testDel]
34733496
class A:

0 commit comments

Comments
 (0)