Updated memory management

changes/256.bugfix.rst

@@ -0,0 +1,2 @@
+Retain Objective-C objects when creating Python wrappers and release them when the
+Python wrapped is garbage collected.

docs/how-to/memory-management.rst

@@ -16,16 +16,17 @@ Rubicon Objective-C operates at runtime, it cannot make use of ARC.
 Reference counting in Rubicon Objective-C
 -----------------------------------------
 
-You won't have to manage reference counts in Python, Rubicon Objective-C will do
-that work for you. It does so by tracking when you gain ownership of an object.
-This is the case when you create an Objective-C instance using a method whose
-name begins with ``alloc``, ``new``, ``copy``, or ``mutableCopy``. Rubicon
-Objective-C will then insert a ``release`` call when the Python variable that
-corresponds to the Objective-C instance is deallocated.
+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
+object when Rubicon creates a ``ObjCInstance`` for it on the Python side, and calling
+``autorelease`` when the ``ObjCInstance`` is garbage collected in Python. Retaining
+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`` calls yourself.
+responsible to insert appropriate ``release`` or ``autorelease`` calls yourself
+to prevent leaking memory.
 
 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

src/rubicon/objc/api.py

@@ -223,12 +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)
-
-        # Mark for release if we acquire ownership of an object. Do not autorelease here because
-        # we might retain a Python reference while the Obj-C reference goes out of scope.
-        if self.name.startswith((b"alloc", b"new", b"copy", b"mutableCopy")):
-            result._needs_release = True
+            result = ObjCInstance(result, returned_from_method=self.name)
 
         return result
 
@@ -783,29 +778,13 @@ def objc_class(self):
             return super(ObjCInstance, type(self)).__getattribute__(self, "_objc_class")
         except AttributeError:
             # This assumes that objects never change their class after they are
-            # seen by Rubicon. There are two reasons why this may not be true:
-            #
-            # 1. Objective-C runtime provides a function object_setClass that
-            #    can change an object's class after creation, and some code
-            #    manipulates objects' isa pointers directly (although the latter
-            #    is no longer officially supported by Apple). This is not
-            #    commonly done in practice, and even then it is usually only
-            #    done during object creation/initialization, so it's basically
-            #    safe to assume that an object's class will never change after
-            #    it's been wrapped in an ObjCInstance.
-            # 2. If a memory address is freed by the Objective-C runtime, and
-            #    then re-allocated by an object of a different type, but the
-            #    Python ObjCInstance wrapper persists, Python will assume the
-            #    object is still of the old type. If a new ObjCInstance wrapper
-            #    for the same pointer is re-created, a check is performed to
-            #    ensure the type hasn't changed; this problem only affects
-            #    pre-existing Python wrappers. If this occurs, it probably
-            #    indicates an issue with the retain count on the Python side (as
-            #    the Objective-C runtime shouldn't be able to dispose of an
-            #    object if Python still has a handle to it). If this *does*
-            #    happen, it will manifest as objects appearing to be the wrong
-            #    type, and/or objects having the wrong list of attributes
-            #    available. Refs #249.
+            # seen by Rubicon. This can occur because the Objective-C runtime provides a
+            # function object_setClass that can change an object's class after creation,
+            # and some code manipulates objects' isa pointers directly (although the
+            # latter is no longer officially supported by Apple). This is not commonly
+            # done in practice, and even then it is usually only done during object
+            # creation/initialization, so it's basically safe to assume that an object's
+            # class will never change after it's been wrapped in an ObjCInstance.
             super(ObjCInstance, type(self)).__setattr__(
                 self, "_objc_class", ObjCClass(libobjc.object_getClass(self))
             )
@@ -815,7 +794,9 @@ def objc_class(self):
     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):
+    def __new__(
+        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`,
         or an existing :class:`ObjCInstance`.
@@ -852,68 +833,17 @@ class or a metaclass, an instance of :class:`ObjCClass` or
                 # If an ObjCInstance already exists for the Objective-C object,
                 # reuse it instead of creating a second ObjCInstance for the
                 # same object.
-                cached = cls._cached_objects[object_ptr.value]
-
-                # In a high-churn environment, it is possible for an object to
-                # be deallocated, and the same memory address be re-used on the
-                # Objective-C side, but the Python wrapper object for the
-                # original instance has *not* been cleaned up. In that
-                # situation, an attempt to wrap the *new* Objective-C object
-                # instance will cause a false positive cache hit; returning a
-                # Python object that has a class that doesn't match the class of
-                # the new instance.
-                #
-                # To prevent this, when we get a cache hit on an ObjCInstance,
-                # use the raw Objective-C API on the pointer to get the current
-                # class of the object referred to by the pointer. If there's a
-                # discrepancy, purge the cache for the memory address, and
-                # re-create the object.
-                #
-                # We do this both when the type *is* ObjCInstance (the case when
-                # instantiating a literal ObjCInstance()), and when type is an
-                # ObjCClass instance (e.g., ObjClass("Example"), which is the
-                # type of a directly instantiated instance of Example.
-                #
-                # We *don't* do this when the type *is* ObjCClass,
-                # ObjCMetaClass, as there's a race condition on startup -
-                # retrieving `.objc_class` causes the creation of ObjCClass
-                # objects, which will cause cache hits trying to re-use existing
-                # ObjCClass objects. However, ObjCClass instances generally
-                # won't be recycled or reused, so that should be safe to exclude
-                # from the cache freshness check.
-                #
-                # One edge case with this approach: if the old and new
-                # Objective-C objects have the same class, they won't be
-                # identified as a stale object, and they'll re-use the same
-                # Python wrapper. This effectively means id(obj) isn't a
-                # reliable instance identifier... but (a) this won't be a common
-                # case; (b) this flaw exists in pure Python and Objective-C as
-                # well, because default object identity is tied to memory
-                # allocation; and (c) the stale wrapper will *work*, because
-                # it's the correct class.
-                #
-                # Refs #249.
-                if cls == ObjCInstance or isinstance(cls, ObjCInstance):
-                    cached_class_name = cached.objc_class.name
-                    current_class_name = libobjc.class_getName(
-                        libobjc.object_getClass(object_ptr)
-                    ).decode("utf-8")
-                    if (
-                        current_class_name != cached_class_name
-                        and not current_class_name.endswith(f"_{cached_class_name}")
-                    ):
-                        # There has been a cache hit, but the object is a
-                        # different class, treat this as a cache miss. We don't
-                        # *just* look for an *exact* class name match, because
-                        # some Cocoa/UIKit classes undergo a class name change
-                        # between `alloc()` and `init()` (e.g., `NSWindow`
-                        # becomes `NSKVONotifying_NSWindow`). Refs #257.
-                        raise KeyError(object_ptr.value)
-
-                return cached
+                return cls._cached_objects[object_ptr.value]
             except KeyError:
                 pass
 
+            # 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(
+                (b"alloc", b"new", b"copy", b"mutableCopy")
+            ):
+                send_message(object_ptr, "retain", restype=objc_id, argtypes=[])
+
             # If the given pointer points to a class, return an ObjCClass instead (if we're not already creating one).
             if not issubclass(cls, ObjCClass) and object_isClass(object_ptr):
                 return ObjCClass(object_ptr)
@@ -932,7 +862,6 @@ class or a metaclass, an instance of :class:`ObjCClass` or
             super(ObjCInstance, type(self)).__setattr__(
                 self, "_as_parameter_", object_ptr
             )
-            super(ObjCInstance, type(self)).__setattr__(self, "_needs_release", False)
             if isinstance(object_ptr, objc_block):
                 super(ObjCInstance, type(self)).__setattr__(
                     self, "block", ObjCBlock(object_ptr)
@@ -944,39 +873,11 @@ class or a metaclass, an instance of :class:`ObjCClass` or
 
         return self
 
-    def release(self):
-        """Manually decrement the reference count of the corresponding objc
-        object.
-
-        The objc object is sent a dealloc message when its reference
-        count reaches 0. Calling this method manually should not be
-        necessary, unless the object was explicitly ``retain``\\ed
-        before. Objects returned from ``.alloc().init...(...)`` and
-        similar calls are released automatically by Rubicon when the
-        corresponding Python object is deallocated.
-        """
-        self._needs_release = False
-        send_message(self, "release", restype=objc_id, argtypes=[])
-
-    def autorelease(self):
-        """Decrements the receiver’s reference count at the end of the current
-        autorelease pool block.
-
-        The objc object is sent a dealloc message when its reference
-        count reaches 0. If called, the object will not be released when
-        the Python object is deallocated.
-        """
-        self._needs_release = False
-        result = send_message(self, "autorelease", restype=objc_id, argtypes=[])
-        return ObjCInstance(result)
-
     def __del__(self):
-        """Release the corresponding objc instance if we own it, i.e., if it
-        was returned by a method starting with :meth:`alloc`, :meth:`new`,
-        :meth:`copy`, or :meth:`mutableCopy` and it wasn't already explicitly
-        released by calling :meth:`release` or :meth:`autorelease`."""
-        if self._needs_release:
-            send_message(self, "release", restype=objc_id, argtypes=[])
+        # 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
+        # it is passed for example to a strong property.
+        send_message(self, "autorelease", restype=objc_id, argtypes=[])
 
     def __str__(self):
         """Get a human-readable representation of ``self``.