[mypyc] Fix memory leak in Exception subclasses with instance attributes#21719
Open
VaggelisD wants to merge 1 commit into
Open
[mypyc] Fix memory leak in Exception subclasses with instance attributes#21719VaggelisD wants to merge 1 commit into
Exception subclasses with instance attributes#21719VaggelisD wants to merge 1 commit into
Conversation
VaggelisD
force-pushed
the
upstream-fix/exception-dict-leak
branch
from
e91081f to
3a568a1
Compare
…e attributes Fixes python#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.
VaggelisD
force-pushed
the
upstream-fix/exception-dict-leak
branch
from
3a568a1 to
05090ce
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 inheritsBaseExceptions, 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.pyso we emit our owntp_dealloc/tp_clear/tp_traversefor 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.