Skip to content

Commit b2dd3bc

Browse files
committed
[mypyc] Fix memory leak when borrowing property getter return values
`is_native_attr_ref()` decides whether an attribute access can safely use a borrowed reference (skipping DECREF). It used `has_attr(name) and not get_method(name)` — the idea being: if there's an attribute but no method, it's a struct field (safe to borrow). However, `has_attr()` checks both `ir.attributes` (struct fields) and `ir.property_types` (properties), and is populated during the preparation phase (always complete). `get_method()` checks `ir.methods`, which is populated during the compilation phase, one module at a time. When modules within an SCC are compiled in nondeterministic order (set iteration of `scc.mod_ids`), `ir.methods` may not be populated for a cross-module class yet. `get_method()` then returns None for a property getter that hasn't been compiled, causing `is_native_attr_ref` to incorrectly return True — treating the property as a plain struct field. Property getters are method calls that return new owned references (the getter INCREFs before returning). Marking the result as borrowed skips the DECREF, leaking one reference per call. The fix checks `ir.attributes` directly (struct fields only, always populated during preparation) instead of the unreliable `has_attr()` + `not get_method()` combination. Properties are never in `ir.attributes`, so they will always correctly return False. Note: the underlying issue is that modules within an SCC are compiled in nondeterministic order, and `ir.methods` is not populated until a module's function bodies are compiled. Any code that reads `ir.methods` during compilation of a different module in the same SCC could have similar order-dependent bugs.
1 parent 818e932 commit b2dd3bc

2 files changed

Lines changed: 45 additions & 2 deletions

File tree

mypyc/irbuild/builder.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,8 +1467,7 @@ def is_native_attr_ref(self, expr: MemberExpr) -> bool:
14671467
return (
14681468
isinstance(obj_rtype, RInstance)
14691469
and obj_rtype.class_ir.is_ext_class
1470-
and obj_rtype.class_ir.has_attr(expr.name)
1471-
and not obj_rtype.class_ir.get_method(expr.name)
1470+
and any(expr.name in ir.attributes for ir in obj_rtype.class_ir.mro)
14721471
)
14731472

14741473
def mark_block_unreachable(self) -> None:

mypyc/test-data/run-classes.test

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6000,3 +6000,47 @@ class InterpGrand(Child):
60006000
ig = InterpGrand(5, 3)
60016001
assert ig.overridden() == -1
60026002
assert ig.calls_overridden() == 0 # -1 + 1
6003+
6004+
[case testPropertyGetterLeak]
6005+
class Bar:
6006+
pass
6007+
6008+
class Foo:
6009+
def __init__(self) -> None:
6010+
self.obj: object = Bar()
6011+
6012+
@property
6013+
def val(self) -> object:
6014+
return self.obj
6015+
6016+
[file other.py]
6017+
import gc
6018+
from native import Foo, Bar
6019+
6020+
def check(foo: Foo) -> bool:
6021+
return isinstance(foo.val, Bar)
6022+
6023+
def test_property_getter_no_leak() -> None:
6024+
foo = Foo()
6025+
gc.collect()
6026+
before = gc.get_objects()
6027+
for _ in range(100):
6028+
check(foo)
6029+
gc.collect()
6030+
after = gc.get_objects()
6031+
diff = len(after) - len(before)
6032+
assert diff <= 2, diff
6033+
6034+
test_property_getter_no_leak()
6035+
6036+
[file driver.py]
6037+
import sys
6038+
from other import check
6039+
from native import Foo
6040+
6041+
foo = Foo()
6042+
init = sys.getrefcount(foo.obj)
6043+
for _ in range(100):
6044+
check(foo)
6045+
after = sys.getrefcount(foo.obj)
6046+
assert after - init == 0, f"Leaked {after - init} refs"

0 commit comments

Comments
 (0)