From 05090cec1de6ba53d176d08a86d1efac4846337f Mon Sep 17 00:00:00 2001 From: vaggelisd Date: Fri, 10 Jul 2026 16:32:36 +0300 Subject: [PATCH] [mypyc] Fix memory leak in Exception and dict subclasses with instance attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- mypyc/codegen/emit.py | 11 +++++++- mypyc/codegen/emitclass.py | 38 ++++++++++++++------------ mypyc/test-data/run-classes.test | 47 ++++++++++++++++++++++++-------- 3 files changed, 66 insertions(+), 30 deletions(-) diff --git a/mypyc/codegen/emit.py b/mypyc/codegen/emit.py index 57cce6a3fe8f..0abf602b1880 100644 --- a/mypyc/codegen/emit.py +++ b/mypyc/codegen/emit.py @@ -1434,8 +1434,17 @@ def emit_cpyfunction_instance( def emit_base_tp_function_call( self, derived_cl: ClassIR, tp_func: str, args: str, *, prefix: str = "" ) -> None: + # Walk past intermediate heap types (Python or mypyc classes) to reach a + # static C-level ancestor. Calling a heap type's tp_dealloc/tp_traverse/ + # tp_clear would dispatch through subtype_dealloc, which uses Py_TYPE(self) + # (still our subtype) and re-enters our own function — infinite recursion. type_obj = self.type_struct_name(derived_cl) - self.emit_line(f"{prefix}{type_obj}->tp_base->{tp_func}({args});") + base_var = f"_base_{tp_func}" + self.emit_line(f"PyTypeObject *{base_var} = {type_obj}->tp_base;") + self.emit_line(f"while ({base_var}->tp_flags & Py_TPFLAGS_HEAPTYPE) {{") + self.emit_line(f" {base_var} = {base_var}->tp_base;") + self.emit_line("}") + self.emit_line(f"{prefix}{base_var}->{tp_func}({args});") def c_array_initializer(components: list[str], *, indented: bool = False) -> str: diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index a1127b15ad9d..3e72f9c653e7 100644 --- a/mypyc/codegen/emitclass.py +++ b/mypyc/codegen/emitclass.py @@ -267,7 +267,16 @@ def generate_class(cl: ClassIR, module: str, emitter: Emitter) -> None: fields["tp_new"] = new_name managed_dict = has_managed_dict(cl, emitter) - if generate_full or managed_dict: + # On Python <3.12, subclasses of builtin types (Exception, dict) get an extra __dict__ + # slot that the inherited base dealloc knows nothing about. Emit our own so it gets freed. + needs_builtin_dict_cleanup = ( + cl.builtin_base is not None + and cl.has_dict + and not managed_dict + and emitter.capi_version < (3, 12) + ) + generate_dealloc_slots = generate_full or managed_dict or needs_builtin_dict_cleanup + if generate_dealloc_slots: fields["tp_dealloc"] = f"(destructor){name_prefix}_dealloc" if not cl.is_acyclic: fields["tp_traverse"] = f"(traverseproc){name_prefix}_traverse" @@ -340,7 +349,7 @@ def emit_line() -> None: else: fields["tp_basicsize"] = base_size - if generate_full or managed_dict: + if generate_dealloc_slots: if not cl.is_acyclic: generate_traverse_for_class(cl, traverse_name, emitter) emit_line() @@ -386,7 +395,8 @@ def emit_line() -> None: emit_line() flags = ["Py_TPFLAGS_DEFAULT", "Py_TPFLAGS_HEAPTYPE", "Py_TPFLAGS_BASETYPE"] - if (generate_full or managed_dict) and not cl.is_acyclic: + if generate_dealloc_slots and not cl.is_acyclic: + # Set explicitly: PyType_Ready won't inherit HAVE_GC once we override tp_dealloc/traverse. flags.append("Py_TPFLAGS_HAVE_GC") if cl.has_method("__call__"): fields["tp_vectorcall_offset"] = "offsetof({}, vectorcall)".format( @@ -882,14 +892,11 @@ def generate_traverse_for_class(cl: ClassIR, func_name: str, emitter: Emitter) - emitter.emit_line(f"rv = PyObject_VisitManagedDict({base_args});") emitter.emit_line("if (rv != 0) return rv;") elif cl.has_dict: - struct_name = cl.struct_name(emitter.names) - # __dict__ lives right after the struct and __weakref__ lives right after that - emitter.emit_gc_visit( - f"*((PyObject **)((char *)self + sizeof({struct_name})))", object_rprimitive - ) + # __dict__ lives at tp_dictoffset (== base_size), __weakref__ right after it. + base_size = f"sizeof({cl.builtin_base or cl.struct_name(emitter.names)})" + emitter.emit_gc_visit(f"*((PyObject **)((char *)self + {base_size}))", object_rprimitive) emitter.emit_gc_visit( - f"*((PyObject **)((char *)self + sizeof(PyObject *) + sizeof({struct_name})))", - object_rprimitive, + f"*((PyObject **)((char *)self + sizeof(PyObject *) + {base_size}))", object_rprimitive ) emitter.emit_line("return rv;") emitter.emit_line("}") @@ -908,14 +915,11 @@ def generate_clear_for_class(cl: ClassIR, func_name: str, emitter: Emitter) -> N if has_managed_dict(cl, emitter): emitter.emit_line(f"PyObject_ClearManagedDict({base_args});") elif cl.has_dict: - struct_name = cl.struct_name(emitter.names) - # __dict__ lives right after the struct and __weakref__ lives right after that - emitter.emit_gc_clear( - f"*((PyObject **)((char *)self + sizeof({struct_name})))", object_rprimitive - ) + # __dict__ lives at tp_dictoffset (== base_size), __weakref__ right after it. + base_size = f"sizeof({cl.builtin_base or cl.struct_name(emitter.names)})" + emitter.emit_gc_clear(f"*((PyObject **)((char *)self + {base_size}))", object_rprimitive) emitter.emit_gc_clear( - f"*((PyObject **)((char *)self + sizeof(PyObject *) + sizeof({struct_name})))", - object_rprimitive, + f"*((PyObject **)((char *)self + sizeof(PyObject *) + {base_size}))", object_rprimitive ) emitter.emit_line("return 0;") emitter.emit_line("}") diff --git a/mypyc/test-data/run-classes.test b/mypyc/test-data/run-classes.test index 56ad3673e289..99698363e6b1 100644 --- a/mypyc/test-data/run-classes.test +++ b/mypyc/test-data/run-classes.test @@ -956,6 +956,40 @@ except Failure as e: assert e.x == 10 heyo() +[case testSubclassExceptionNoAttributeLeak] +# Regression test: on Python <3.12, a mypyc-compiled Exception subclass with +# instance attributes used to leak its __dict__ contents on every destruction +# because BaseException's inherited dealloc/clear doesn't know about the extra +# __dict__ slot mypyc appends at tp_dictoffset = sizeof(PyBaseExceptionObject). +class LeakyErr(Exception): + def __init__(self, payload: object) -> None: + self.data = payload + +[file driver.py] +import gc +from native import LeakyErr + +class Payload: + live = 0 + def __init__(self) -> None: + Payload.live += 1 + def __del__(self) -> None: + Payload.live -= 1 + +# Each iteration creates a fresh Payload, stashes it on a fresh LeakyErr, and +# drops both. If the exception's __dict__ leaks, the Payload it references +# survives — __del__ never runs and Payload.live grows without bound. +for _ in range(1000): + LeakyErr(Payload()) +gc.collect() +assert Payload.live == 0, f"leaked {Payload.live} payloads" + +# Sanity-check the normal raise/catch path still works and attrs are readable. +try: + raise LeakyErr("hi") +except LeakyErr as e: + assert e.data == "hi" + [case testSubclassDict] from typing import Dict class WelpDict(Dict[str, int]): @@ -3452,22 +3486,11 @@ def test_dict_subclass_dealloc() -> None: del d [file driver.py] -import sys - from native import events, test_dict_subclass_dealloc test_dict_subclass_dealloc() -expected_events: list[str] = [] - -# TODO: Fix when compiling for older python. -# The user-defined __del__ method is currently only invoked when __dict__ is a managed dict -# because calling __del__ in tp_clear on older python crashes. -if sys.version_info >= (3, 12): - expected_events.append("deleting DictSubclass") -expected_events.append("deleting Item") - -assert events == expected_events, events +assert events == ["deleting DictSubclass", "deleting Item"], events [case testDel] class A: