Skip to content

Commit 05090ce

Browse files
committed
[mypyc] Fix memory leak in Exception and dict subclasses with instance attributes
Fixes #21716. When mypyc compiles a subclass of a builtin type (`Exception` or `dict`) with instance attributes on Python <3.12, it appends a __dict__ slot at tp_dictoffset = sizeof(builtin_base) to hold them. But it emits no subtype-specific tp_dealloc, so the type inherits the base's dealloc which knows nothing about the extra slot. That dict (and everything it references) leaks on every instance destruction. Hit downstream in sqlglot[c] (tobymao/sqlglot#7853) where every raised-and-caught ParseError leaked ~1 KB. On 3.12+ mypyc already avoids adding the extra offset, so only <3.12 is affected. Changes in `mypyc/codegen/emitclass.py`: * Extend the gate that emits tp_dealloc/tp_clear/tp_traverse (and Py_TPFLAGS_HAVE_GC) to fire for any builtin_base subclass with has_dict under capi < 3.12. The existing builtin-base dealloc dance handles the flow. * 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; once we supply our own, that inheritance is lost. * Fix a latent offset bug in generate_traverse_for_class / generate_clear_for_class: they used sizeof(struct_name) for the extra dict slot, but for builtin_base classes tp_dictoffset is sizeof(builtin_base). Previously unreachable because the emission gate never fired for builtin_base classes. Changes in `mypyc/codegen/emit.py` (`emit_base_tp_function_call`): * Walk the tp_base chain past any Py_TPFLAGS_HEAPTYPE ancestors before delegating tp_dealloc/tp_traverse/tp_clear. Required for Exception subclasses that inherit through an interpreted Python heap type (e.g. `argparse.ArgumentTypeError` in mypy's own VersionTypeError): calling the heap type's tp_dealloc dispatches through CPython's subtype_dealloc, which reads Py_TYPE(self) (still our subtype), finds our own tp_dealloc, and calls it back — infinite recursion crashing the compiled binary on 3.10/3.11. Our subtype_clear already walks the full MRO and clears all ancestors' fields, so calling the static ancestor's tp_dealloc directly is safe and correct. Regression tests: * testSubclassExceptionNoAttributeLeak (new): a Payload class with a __del__ counter detects the leak deterministically — 1000 fresh exceptions each hold a fresh payload; without the fix all 1000 payloads survive. * testDelForDictSubclass (updated): the stale `sys.version_info >= (3, 12)` guard is dropped since dict subclass __del__ now works correctly on all Python versions.
1 parent d5daed2 commit 05090ce

3 files changed

Lines changed: 66 additions & 30 deletions

File tree

mypyc/codegen/emit.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1434,8 +1434,17 @@ def emit_cpyfunction_instance(
14341434
def emit_base_tp_function_call(
14351435
self, derived_cl: ClassIR, tp_func: str, args: str, *, prefix: str = ""
14361436
) -> None:
1437+
# Walk past intermediate heap types (Python or mypyc classes) to reach a
1438+
# static C-level ancestor. Calling a heap type's tp_dealloc/tp_traverse/
1439+
# tp_clear would dispatch through subtype_dealloc, which uses Py_TYPE(self)
1440+
# (still our subtype) and re-enters our own function — infinite recursion.
14371441
type_obj = self.type_struct_name(derived_cl)
1438-
self.emit_line(f"{prefix}{type_obj}->tp_base->{tp_func}({args});")
1442+
base_var = f"_base_{tp_func}"
1443+
self.emit_line(f"PyTypeObject *{base_var} = {type_obj}->tp_base;")
1444+
self.emit_line(f"while ({base_var}->tp_flags & Py_TPFLAGS_HEAPTYPE) {{")
1445+
self.emit_line(f" {base_var} = {base_var}->tp_base;")
1446+
self.emit_line("}")
1447+
self.emit_line(f"{prefix}{base_var}->{tp_func}({args});")
14391448

14401449

14411450
def c_array_initializer(components: list[str], *, indented: bool = False) -> str:

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)