Skip to content

Commit 82e8498

Browse files
authored
[mypyc] Fix memory leak in Exception subclasses with instance attributes (#21719)
Fixes #21716 When mypyc compiles `class Foo(Exception)` with instance attributes, it staples an extra `__dict__` slot onto the end of the base exception struct to hold them but it doesn't generate its own dealloc; the type inherits `BaseException`s, which only frees the base's own fields and has no idea the extra slot exists, meaning that the dict leaks on every destruction. The fix: Flip the gate in `emitclass.py` so we emit our own `tp_dealloc` / `tp_clear` / `tp_traverse` for this case too. The existing builtin-base dealloc path then does the right thing: our clear frees the extra slot, then the base's dealloc frees the rest.
1 parent 2e6c87e commit 82e8498

3 files changed

Lines changed: 103 additions & 31 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: 30 additions & 18 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
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)
913921
emitter.emit_gc_clear(
914-
f"*((PyObject **)((char *)self + sizeof({struct_name})))", object_rprimitive
915-
)
916-
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("}")
@@ -931,7 +935,13 @@ def generate_dealloc_for_class(
931935
emitter.emit_line("static void")
932936
emitter.emit_line(f"{dealloc_func_name}({cl.struct_name(emitter.names)} *self)")
933937
emitter.emit_line("{")
934-
if has_tp_finalize:
938+
# Always run the finalizer dance for builtin_base subclasses: we're bypassing
939+
# subtype_dealloc, so an inherited tp_finalize (e.g. from an interpreted base's
940+
# __del__) would otherwise be skipped. Runtime-gate on tp_finalize so we no-op
941+
# when nothing in the MRO defines a finalizer.
942+
if has_tp_finalize or cl.builtin_base:
943+
if not has_tp_finalize:
944+
emitter.emit_line("if (Py_TYPE(self)->tp_finalize) {")
935945
emitter.emit_line("PyObject *type, *value, *traceback;")
936946
emitter.emit_line("PyErr_Fetch(&type, &value, &traceback);")
937947
emitter.emit_line("int res = PyObject_CallFinalizerFromDealloc((PyObject *)self);")
@@ -948,6 +958,8 @@ def generate_dealloc_for_class(
948958
emitter.emit_line("if (res < 0) {")
949959
emitter.emit_line("goto done;")
950960
emitter.emit_line("}")
961+
if not has_tp_finalize:
962+
emitter.emit_line("}")
951963
if not cl.is_acyclic:
952964
emitter.emit_line("PyObject_GC_UnTrack(self);")
953965
if cl.builtin_base:

mypyc/test-data/run-classes.test

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,68 @@ 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+
993+
[case testInheritedHeapBaseFinalizerRuns]
994+
# Regression test: when a mypyc-compiled Exception subclass inherits from an
995+
# interpreted (heap) base that defines __del__, the base's finalizer must still
996+
# run. Our custom tp_dealloc bypasses subtype_dealloc, so we must invoke
997+
# PyObject_CallFinalizerFromDealloc ourselves when tp_finalize is inherited.
998+
from mypy_extensions import mypyc_attr
999+
1000+
events: list[str] = []
1001+
1002+
@mypyc_attr(native_class=False)
1003+
class HeapError(Exception):
1004+
def __del__(self) -> None:
1005+
events.append("base del")
1006+
1007+
class NativeError(HeapError):
1008+
def __init__(self, payload: object) -> None:
1009+
self.own = payload
1010+
1011+
[file driver.py]
1012+
import gc
1013+
from native import NativeError, events
1014+
1015+
for _ in range(1):
1016+
NativeError(object())
1017+
1018+
gc.collect()
1019+
assert events == ["base del"], events
1020+
9591021
[case testSubclassDict]
9601022
from typing import Dict
9611023
class WelpDict(Dict[str, int]):
@@ -3452,22 +3514,11 @@ def test_dict_subclass_dealloc() -> None:
34523514
del d
34533515

34543516
[file driver.py]
3455-
import sys
3456-
34573517
from native import events, test_dict_subclass_dealloc
34583518

34593519
test_dict_subclass_dealloc()
34603520

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
3521+
assert events == ["deleting DictSubclass", "deleting Item"], events
34713522

34723523
[case testDel]
34733524
class A:

0 commit comments

Comments
 (0)