[mypyc] Fix memory leak on property setter call (#21095)

Calling a property setter method results in a memory leak because the
setter method internally increfs the passed value and there is no
corresponding decref on the caller side. This happens because the
`SetAttr` op treats the source value as stolen which makes sense for
regular attribute set.

Change the op to not treat the source value as stolen for sets that
call property setter methods so a decref is automatically generated
after calling the setter.

The leak can be observed by manually looking at memory use in
`top` when running a loop like this one:

```python
class Foo():
    def __init__(self) -> None:
        self.attr = "unmodified"

class A:
    def __init__(self) -> None:
        self._foo = Foo()

    @Property
    def foo(self) -> Foo:
        return self._foo

    @foo.setter
    def foo(self, val : Foo) -> None:
        self._foo = val

a = A()
while True:
    a.foo = Foo()
```

Author: p-sawicki

mypyc/codegen/emitfunc.py

@@ -484,7 +484,7 @@ def visit_set_attr(self, op: SetAttr) -> None:
         rtype = op.class_type
         cl = rtype.class_ir
         attr_rtype, decl_cl = cl.attr_details(op.attr)
-        if cl.get_method(op.attr):
+        if op.is_propset:
             # Again, use vtable access for properties...
             assert not op.is_init and op.error_kind == ERR_FALSE, "%s %d %d %s" % (
                 op.attr,

mypyc/ir/ops.py

@@ -31,6 +31,7 @@ class to enable the new behavior. Sometimes adding a new abstract
 
 from mypy_extensions import trait
 
+from mypyc.common import PROPSET_PREFIX
 from mypyc.ir.deps import Dependency
 from mypyc.ir.rtypes import (
     RArray,
@@ -909,10 +910,7 @@ def accept(self, visitor: OpVisitor[T]) -> T:
 
 @final
 class SetAttr(RegisterOp):
-    """obj.attr = src (for a native object)
-
-    Steals the reference to src.
-    """
+    """obj.attr = src (for a native object)"""
 
     error_kind = ERR_FALSE
 
@@ -928,6 +926,16 @@ def __init__(self, obj: Value, attr: str, src: Value, line: int) -> None:
         # and we don't use a setter
         self.is_init = False
 
+        cl = self.class_type.class_ir
+        is_propset = False
+        for ir in cl.mro:
+            propset = ir.method_decls.get(PROPSET_PREFIX + attr)
+            if propset is not None:
+                is_propset = not propset.implicit
+                break
+        # If True, this op represents calling a property setter.
+        self.is_propset = is_propset
+
     def mark_as_initializer(self) -> None:
         self.is_init = True
         self.error_kind = ERR_NEVER
@@ -940,6 +948,10 @@ def set_sources(self, new: list[Value]) -> None:
         self.obj, self.src = new
 
     def stolen(self) -> list[Value]:
+        # The property setter method increfs the passed value so don't treat it as a steal
+        # to avoid leaking.
+        if self.is_propset:
+            return []
         return [self.src]
 
     def accept(self, visitor: OpVisitor[T]) -> T:

mypyc/test-data/refcount.test

@@ -1931,3 +1931,131 @@ L0:
     r10 = unborrow r8
     x = r10
     return x
+
+[case testPropertySetterCallWithRefcountedObject]
+class Foo:
+    def __init__(self) -> None:
+        self.attr = "unmodified"
+
+class A:
+    def __init__(self) -> None:
+        self._foo = Foo()
+
+    @property
+    def foo(self) -> Foo:
+        return self._foo
+
+    @foo.setter
+    def foo(self, val : Foo) -> None:
+        self._foo = val
+
+def f(a: A):
+    a.foo = Foo()
+[out]
+def Foo.__init__(self):
+    self :: __main__.Foo
+    r0 :: str
+L0:
+    r0 = 'unmodified'
+    inc_ref r0
+    self.attr = r0
+    return 1
+def A.__init__(self):
+    self :: __main__.A
+    r0 :: __main__.Foo
+L0:
+    r0 = Foo()
+    self._foo = r0
+    return 1
+def A.foo(self):
+    self :: __main__.A
+    r0 :: __main__.Foo
+L0:
+    r0 = self._foo
+    return r0
+def A.__mypyc_setter__foo(self, val):
+    self :: __main__.A
+    val :: __main__.Foo
+    r0 :: bool
+L0:
+    inc_ref val
+    self._foo = val; r0 = is_error
+    return 1
+def f(a):
+    a :: __main__.A
+    r0 :: __main__.Foo
+    r1 :: bool
+    r2 :: object
+L0:
+    r0 = Foo()
+    a.foo = r0; r1 = is_error
+    dec_ref r0
+    r2 = box(None, 1)
+    inc_ref r2
+    return r2
+
+[case testPropertySetterCallWithNonRefcountedObject_64bit]
+from mypy_extensions import i64
+
+class A:
+    def __init__(self) -> None:
+        self._foo = i64(0)
+
+    @property
+    def foo(self) -> i64:
+        return self._foo
+
+    @foo.setter
+    def foo(self, val : i64) -> None:
+        self._foo = val
+
+def f(a: A, i: int):
+    a.foo = i64(i)
+[out]
+def A.__init__(self):
+    self :: __main__.A
+L0:
+    self._foo = 0
+    return 1
+def A.foo(self):
+    self :: __main__.A
+    r0 :: i64
+L0:
+    r0 = self._foo
+    return r0
+def A.__mypyc_setter__foo(self, val):
+    self :: __main__.A
+    val :: i64
+    r0 :: bool
+L0:
+    self._foo = val; r0 = is_error
+    return 1
+def f(a, i):
+    a :: __main__.A
+    i :: int
+    r0 :: native_int
+    r1 :: bit
+    r2, r3 :: i64
+    r4 :: ptr
+    r5 :: c_ptr
+    r6 :: i64
+    r7 :: bool
+    r8 :: object
+L0:
+    r0 = i & 1
+    r1 = r0 == 0
+    if r1 goto L1 else goto L2 :: bool
+L1:
+    r2 = i >> 1
+    r3 = r2
+    goto L3
+L2:
+    r4 = i ^ 1
+    r5 = r4
+    r6 = CPyLong_AsInt64(r5)
+    r3 = r6
+L3:
+    a.foo = r3; r7 = is_error
+    r8 = box(None, 1)
+    inc_ref r8
+    return r8

mypyc/test-data/run-classes.test

@@ -2160,8 +2160,40 @@ class H:
 
 [file other.py]
 # Run in both interpreted and compiled mode
+import gc
+
+from native import A, B, C, D, E, F, Foo, G
+
+def test_leaks() -> None:
+    a = A()
+
+    gc.collect()
+    before = gc.get_objects()
+    for _ in range(100):
+        a.foo = Foo()
+    gc.collect()
+    after = gc.get_objects()
+
+    diff = len(after) - len(before)
+    # Small difference is expected, if the property setter call was leaking objects
+    # we would get a difference similar to the number of iterations above.
+    assert diff <= 2, diff
 
-from native import A, B, C, D, E, F, G
+def set_foo(foo: Foo) -> None:
+    a = A()
+    a.foo = foo
+
+def test_use_after_passing_to_prop_setter() -> None:
+    foo = Foo()
+
+    foo.attr = "before"
+    assert foo.attr == "before", foo.attr
+
+    set_foo(foo)
+    assert foo.attr == "before", foo.attr
+
+    foo.attr = "after"
+    assert foo.attr == "after", foo.attr
 
 a = A()
 assert a.x == 0
@@ -2195,6 +2227,9 @@ g = G(4)
 g.x = 20
 assert g.x == 20
 
+test_leaks()
+test_use_after_passing_to_prop_setter()
+
 [file driver.py]
 # Run the tests in both interpreted and compiled mode
 import other