[mypyc] Fix @property getter memory leak#21230
Open
VaggelisD wants to merge 1 commit intopython:masterfrom
Open
[mypyc] Fix @property getter memory leak#21230VaggelisD wants to merge 1 commit intopython:masterfrom
@property getter memory leak#21230VaggelisD wants to merge 1 commit intopython:masterfrom
Conversation
`is_native_attr_ref()` uses `has_attr(name) and not get_method(name)` to decide if an attribute access can borrow. `has_attr()` is populated during preparation (always complete), but `get_method()` checks `ir.methods` which is populated during compilation. When cross-module classes haven't been compiled yet, `get_method()` returns None for property getters, incorrectly allowing borrowing. Property getters return new owned references, so skipping the DECREF leaks one reference per call. The fix checks `ir.attributes` directly (struct fields only, always populated during preparation). Properties are never in `ir.attributes`, so they always return False. This is the getter-side counterpart to python#21095.
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
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.
Issue
Mypyc can incorrectly borrow references from property getters (owned references).
The borrow mechanism is safe for struct field access since they return a pointer into the object's memory; Property getters on the other hand are method calls that return new owned references i.e the caller must
DECREFthem. If mypyc mistakes a property for a struct field, it skips theDECREF, leaking one reference per call.I think this affects any expression context that enables borrowing (comparisons,
isinstance,is Nonechecks) when the borrowed expression is a property access on a cross-module class. We discovered this through OOM issues in SQLGlot, whereisinstance(self.this, Func)(thisis a@property) leaked on every call.Repro
Compile both with mypyc passing in
PYTHONHASHSEED=3printsLeaked refs: 100.Root cause
is_native_attr_ref()decides if an attribute access can safely borrow by checkinghas_attr(name) and not get_method(name)i.e "if there's an attribute but no method, it's a struct field".has_attr()is always fully populated,get_method()depends on whether the class's module (basein this case) has been compiled yet.get_method()returnsNone->is_native_attr_refincorrectly treats the property as a borrowed struct field.I think there's a broader issue here: Even when
derived.pyimports frombase.py(no cycle), mypyc can place them in the same SCC and compile derived before base. Sinceir.methodsis only populated when a module's function bodies are compiled, any code that readsir.methods(i.eget_methods) during compilation of a different module in the same SCC could have similar order-dependent bugs.The suggested fix
Check
ir.attributesdirectly (struct fields only, always populated) instead of the unreliableget_method. I believe this is the getter-side counterpart to #21095 which fixed the same class of bug for property setters.