gui: Fix issues with binding class methods on children

Author: eruvanos

arcade/gui/property.py

@@ -1,9 +1,12 @@
 import inspect
 import sys
 import traceback
+import warnings
+import weakref
 from collections.abc import Callable
 from contextlib import contextmanager, suppress
 from enum import Enum
+from inspect import ismethod
 from typing import Any, Generic, TypeVar, cast
 from weakref import WeakKeyDictionary, ref
 
@@ -221,7 +224,52 @@ def __set__(self, instance, value: P):
         self.set(instance, value)
 
 
-def bind(instance, property: str, callback: AnyListener):
+class _WeakCallback:
+    """Wrapper for weakly referencing a callback function.
+
+    Which allows to bind methods of the instance itself without
+    causing memory leaks.
+
+    Also supports to be stored in a dict or set, because it implements
+    __hash__ and __eq__ methods to match the original function.
+    """
+
+    def __init__(self, func):
+        self._func_type = _ListenerType.detect_callback_type(func)  # type: ignore[assignment]
+        self._hash = hash(func)
+
+        if inspect.ismethod(func):
+            self._func = weakref.WeakMethod(func)
+        else:
+            self._func = weakref.ref(func)
+
+    def __call__(self, instance, new_value, old_value):
+        func = self._func()
+        if func is None:
+            warnings.warn("WeakCallable was called without a callable object.")
+
+        if self._func_type == _ListenerType.NO_ARG:
+            return func()
+        elif self._func_type == _ListenerType.INSTANCE:
+            return func(instance)
+        elif self._func_type == _ListenerType.INSTANCE_VALUE:
+            return func(instance, new_value)
+        elif self._func_type == _ListenerType.INSTANCE_NEW_OLD:
+            return func(instance, new_value, old_value)
+
+        else:
+            raise TypeError(f"Unsupported callback type: {self._func_type}")
+
+    def __hash__(self):
+        return self._hash
+
+    def __eq__(self, other):
+        if ismethod(other):
+            return self._hash == hash(other)
+        return False
+
+
+def bind(instance, property: str, callback: AnyListener, weak=False):
     """Bind a function to the change event of the property.
 
     A reference to the function will be kept, so that it will be still
@@ -243,17 +291,24 @@ class MyObject:
 
     Binding to a method of the Property owner itself can cause a memory leak, because the
     owner is strongly referenced. Instead, bind the class method, which will be invoked with
-    the instance as first parameter.
-
+    the instance as first parameter. `bind(instance, "property_name", Instance.method)`.
+    Or use the `weak` parameter to bind the method weakly `bind(instance, "property_name", instance.method, weak=True)`
 
     Args:
         instance: Instance owning the property
         property: Name of the property
         callback: Function to call
+        weak: If True, the callback will be weakly referenced.
+            This is useful for methods of the instance itself to avoid memory leaks.
 
     Returns:
         None
     """
+
+    if weak:
+        # If weak is True, we use a _WeakCallable to avoid strong references
+        callback = _WeakCallback(callback)  # type: ignore[assignment]
+
     # TODO rename property to property_name for arcade 4.0 (just to be sure)
     t = type(instance)
     prop = getattr(t, property)

arcade/gui/widgets/layout.py

@@ -288,23 +288,23 @@ def add(self, child: W, **kwargs) -> W:
             child: The widget to add to the layout.
         """
         # subscribe to child's changes, which might affect the own size hint
-        bind(child, "_children", UIBoxLayout._trigger_size_hint_update)
-        bind(child, "rect", UIBoxLayout._trigger_size_hint_update)
-        bind(child, "size_hint", UIBoxLayout._trigger_size_hint_update)
-        bind(child, "size_hint_min", UIBoxLayout._trigger_size_hint_update)
-        bind(child, "size_hint_max", UIBoxLayout._trigger_size_hint_update)
+        bind(child, "_children", self._trigger_size_hint_update, weak=True)
+        bind(child, "rect", self._trigger_size_hint_update, weak=True)
+        bind(child, "size_hint", self._trigger_size_hint_update, weak=True)
+        bind(child, "size_hint_min", self._trigger_size_hint_update, weak=True)
+        bind(child, "size_hint_max", self._trigger_size_hint_update, weak=True)
 
         return super().add(child, **kwargs)
 
     @override
     def remove(self, child: UIWidget):
         """Remove a child from the layout."""
         # unsubscribe from child's changes
-        unbind(child, "_children", UIBoxLayout._trigger_size_hint_update)
-        unbind(child, "rect", UIBoxLayout._trigger_size_hint_update)
-        unbind(child, "size_hint", UIBoxLayout._trigger_size_hint_update)
-        unbind(child, "size_hint_min", UIBoxLayout._trigger_size_hint_update)
-        unbind(child, "size_hint_max", UIBoxLayout._trigger_size_hint_update)
+        unbind(child, "_children", self._trigger_size_hint_update)
+        unbind(child, "rect", self._trigger_size_hint_update)
+        unbind(child, "size_hint", self._trigger_size_hint_update)
+        unbind(child, "size_hint_min", self._trigger_size_hint_update)
+        unbind(child, "size_hint_max", self._trigger_size_hint_update)
 
         return super().remove(child)
 
@@ -547,11 +547,11 @@ def add(
             row_span: Number of rows the widget will stretch for.
         """
         # subscribe to child's changes, which might affect the own size hint
-        bind(child, "_children", UIGridLayout._trigger_size_hint_update)
-        bind(child, "rect", UIGridLayout._trigger_size_hint_update)
-        bind(child, "size_hint", UIGridLayout._trigger_size_hint_update)
-        bind(child, "size_hint_min", UIGridLayout._trigger_size_hint_update)
-        bind(child, "size_hint_max", UIGridLayout._trigger_size_hint_update)
+        bind(child, "_children", self._trigger_size_hint_update, weak=True)
+        bind(child, "rect", self._trigger_size_hint_update, weak=True)
+        bind(child, "size_hint", self._trigger_size_hint_update, weak=True)
+        bind(child, "size_hint_min", self._trigger_size_hint_update, weak=True)
+        bind(child, "size_hint_max", self._trigger_size_hint_update, weak=True)
 
         return super().add(
             child,

tests/unit/gui/test_property.py

@@ -144,12 +144,44 @@ def test_bind_callback_with_star_args():
     observer.call_args = None
 
 
-def test_unbind_callback():
+def test_unbind_function_callback():
+    called = False
+    def callback(*args):
+        nonlocal called
+        called = True
+
+    my_obj = MyObject()
+    bind(my_obj, "name", callback)
+
+    # WHEN
+    unbind(my_obj, "name", callback)
+    my_obj.name = "New Name"
+
+    assert not called
+    
+def test_unbind_method_callback():
     observer = Observer()
 
     my_obj = MyObject()
     bind(my_obj, "name", observer.call)
 
+    gc.collect()
+
+    # WHEN
+    unbind(my_obj, "name", observer.call)
+    my_obj.name = "New Name"
+
+    assert not observer.called
+
+
+def test_unbind_weak_method_callback():
+    observer = Observer()
+
+    my_obj = MyObject()
+    bind(my_obj, "name", observer.call, weak=True)
+
+    gc.collect()
+
     # WHEN
     unbind(my_obj, "name", observer.call)
     my_obj.name = "New Name"