Updated memory management

changes/256.bugfix.rst

@@ -1,2 +1,10 @@
 Retain Objective-C objects when creating Python wrappers and release them when the
-Python wrapped is garbage collected.
+Python wrapped is garbage collected. This means that manual ``retain`` calls and
+subsequent ``release`` or ``autorelease`` calls from Python are no longer needed with
+very few exceptions such as:
+
+1. When implementing methods like ``copy`` that are supposed to create an object, if
+   the returned object is not actually newly created.
+2. When dealing with side effects of methods like ``init`` that may release an object
+   which is still referenced from Python. See for example
+   https://github.com/beeware/toga/issues/2468.

changes/256.removal.rst

@@ -0,0 +1,4 @@
+Rubicon no longer skips releasing an Objective-C objects when its Python wrapped is
+garbage collected. This means that fewer ``retain`` than ``release`` calls will cause
+segfaults on garbage collection. Review your code carefully for unbalanced ``retain``
+and ``release`` calls before updating.

docs/how-to/memory-management.rst

@@ -2,6 +2,9 @@
 Memory management for Objective-C instances
 ===========================================
 
+Reference counting in Objective-C
+=================================
+
 Reference counting works differently in Objective-C compared to Python. Python
 will automatically track where variables are referenced and free memory when
 the reference count drops to zero whereas Objective-C uses explicit reference
@@ -13,8 +16,8 @@ When enabling automatic reference counting (ARC), the appropriate calls for
 memory management will be inserted for you at compile-time. However, since
 Rubicon Objective-C operates at runtime, it cannot make use of ARC.
 
-Reference counting in Rubicon Objective-C
------------------------------------------
+Reference management in Rubicon
+===============================
 
 In most cases, you won't have to manage reference counts in Python, Rubicon
 Objective-C will do that work for you. It does so by calling ``retain`` on an
@@ -23,19 +26,26 @@ object when Rubicon creates a ``ObjCInstance`` for it on the Python side, and ca
 the object ensures it is not deallocated while it is still referenced from Python
 and releasing it again on ``__del__`` ensures that we do not leak memory.
 
-An exception to this is when you manually ``retain`` an object. Rubicon
-Objective-C will not keep track of such retain calls and you will be
-responsible to insert appropriate ``release`` or ``autorelease`` calls yourself
-to prevent leaking memory.
+The only exception to this is when you create an object -- which is always done
+through methods starting with "alloc", "new", "copy", or "mutableCopy". Rubicon does
+not explicitly retain such objects because we own objects created by us, but Rubicon
+does autorelease them when the Python wrapper is garbage collected.
+
+Rubicon Objective-C will not keep track if you additionally manually ``retain`` an
+object. You will be responsible to insert appropriate ``release`` or ``autorelease``
+calls yourself to prevent leaking memory.
+
+Weak references in Objective-C
+------------------------------
 
-You will also need to pay attention to reference counting in case of **weak
-references**. In Objective-C, creating a **weak reference** means that the
-reference count of the object is not incremented and the object will still be
+You will need to pay attention to reference counting in case of **weak
+references**. In Objective-C, as in Python, creating a weak reference means that
+the reference count of the object is not incremented and the object will be
 deallocated when no strong references remain. Any weak references to the object
 are then set to ``nil``.
 
-Some objects will store references to other objects as a weak reference. Such
-properties will be declared in the Apple developer documentation as
+Some Objective-C objects store references to other objects as a weak reference.
+Such properties will be declared in the Apple developer documentation as
 "@property(weak)" or "@property(assign)". This is commonly the case for
 delegates. For example, in the code below, the ``NSOutlineView`` only stores a
 weak reference to the object which is assigned to its delegate property:

docs/spelling_wordlist

@@ -1,6 +1,9 @@
 Alea
+alloc
 Autorelease
+autorelease
 autoreleased
+autoreleases
 Bugfixes
 callables
 CPython
@@ -22,6 +25,7 @@ lookups
 macOS
 metaclass
 metaclasses
+mutableCopy
 namespace
 namespaces
 ObjC

src/rubicon/objc/api.py

@@ -223,7 +223,7 @@ def __call__(self, receiver, *args, convert_args=True, convert_result=True):
 
         # Convert result to python type if it is an instance or class pointer.
         if self.restype is not None and issubclass(self.restype, objc_id):
-            result = ObjCInstance(result, returned_from_method=self.name)
+            result = ObjCInstance(result, _returned_from_method=self.name)
 
         return result
 
@@ -795,7 +795,7 @@ def _associated_attr_key_for_name(name):
         return SEL(f"rubicon.objc.py_attr.{name}")
 
     def __new__(
-        cls, object_ptr, _name=None, _bases=None, _ns=None, returned_from_method=b""
+        cls, object_ptr, _name=None, _bases=None, _ns=None, _returned_from_method=b""
     ):
         """The constructor accepts an :class:`~rubicon.objc.runtime.objc_id` or
         anything that can be cast to one, such as a :class:`~ctypes.c_void_p`,
@@ -814,10 +814,17 @@ class or a metaclass, an instance of :class:`ObjCClass` or
         :func:`register_type_for_objcclass`. Creating an :class:`ObjCInstance`
         from a ``nil`` pointer returns ``None``.
 
-        Rubicon currently does not perform any automatic memory management on
-        the Objective-C object wrapped in an :class:`ObjCInstance`. It is the
-        user's responsibility to ``retain`` and ``release`` wrapped objects as
-        needed, like in Objective-C code without automatic reference counting.
+        Rubicon retains an Objective-C object when it is wrapped in an
+        :class:`ObjCInstance` and autoreleases it when the :class:`ObjCInstance` is
+        garbage collected.
+
+        The only exception to this are objects returned by methods which create an
+        object (starting with "alloc", "new", "copy", or "mutableCopy"). We do not
+        explicitly retain them because we already own objects created by us, but we do
+        autorelease them on garbage collection of the Python wrapper.
+
+        This ensures that the :class:`ObjCInstance` can always be  used from Python
+        without segfaults while preventing Rubicon from leaking memory.
         """
 
         # Make sure that object_ptr is wrapped in an objc_id.
@@ -839,7 +846,7 @@ class or a metaclass, an instance of :class:`ObjCClass` or
 
             # Explicitly retain the instance on first handover to Python unless we
             # received it from a method that gives us ownership already.
-            if not returned_from_method.startswith(
+            if not _returned_from_method.startswith(
                 (b"alloc", b"new", b"copy", b"mutableCopy")
             ):
                 send_message(object_ptr, "retain", restype=objc_id, argtypes=[])
@@ -875,7 +882,7 @@ class or a metaclass, an instance of :class:`ObjCClass` or
 
     def __del__(self):
         # Autorelease our reference on garbage collection of the Python wrapper. We use
-        # release instead of autorelease allow ObjC to take ownership of an object when
+        # autorelease instead of release to allow ObjC to take ownership of an object when
         # it is passed for example to a strong property.
         send_message(self, "autorelease", restype=objc_id, argtypes=[])
 

tests/test_core.py

@@ -1914,35 +1914,39 @@ def test_objcinstance_explicit_retained_gc_released(self):
         # Assert that the obj was deallocated.
         self.assertIsNone(wr.weak_property)
 
-    # def test_objcinstance_dealloc(self):
-    #     class DeallocTester(NSObject):
-    #         attr0 = objc_property()
-    #         attr1 = objc_property(weak=True)
-    #
-    #         @objc_method
-    #         def dealloc(self):
-    #             self._did_dealloc = True
-    #
-    #     obj = DeallocTester.alloc().init()
-    #     obj.__dict__["_did_dealloc"] = False
-    #
-    #     attr0 = NSObject.alloc().init()
-    #     attr1 = NSObject.alloc().init()
-    #
-    #     obj.attr0 = attr0
-    #     obj.attr1 = attr1
-    #
-    #     self.assertEqual(attr0.retainCount(), 2)
-    #     self.assertEqual(attr1.retainCount(), 1)
-    #
-    #     # ObjC object will be deallocated, can only access Python attributes afterwards.
-    #     obj.release()
-    #
-    #     self.assertTrue(obj._did_dealloc, "custom dealloc did not run")
-    #     self.assertEqual(
-    #         attr0.retainCount(), 1, "strong property value was not released"
-    #     )
-    #     self.assertEqual(attr1.retainCount(), 1, "weak property value was released")
+    def test_objcinstance_dealloc(self):
+
+        class DeallocTester(NSObject):
+            did_dealloc = False
+
+            attr0 = objc_property()
+            attr1 = objc_property(weak=True)
+
+            @objc_method
+            def dealloc(self):
+                DeallocTester.did_dealloc = True
+
+        obj = DeallocTester.alloc().init()
+
+        attr0 = NSObject.alloc().init()
+        attr1 = NSObject.alloc().init()
+
+        obj.attr0 = attr0
+        obj.attr1 = attr1
+
+        self.assertEqual(attr0.retainCount(), 2)
+        self.assertEqual(attr1.retainCount(), 1)
+
+        # ObjC object will be deallocated, can only access Python attributes afterwards.
+        with autoreleasepool():
+            del obj
+            gc.collect()
+
+        self.assertTrue(DeallocTester.did_dealloc, "custom dealloc did not run")
+        self.assertEqual(
+            attr0.retainCount(), 1, "strong property value was not released"
+        )
+        self.assertEqual(attr1.retainCount(), 1, "weak property value was released")
 
     def test_partial_with_override(self):
         """If one method in a partial is overridden, that doesn't impact lookup of other partial targets"""