diff --git a/arcade/gui/property.py b/arcade/gui/property.py index 400536f9b8..b446a34404 100644 --- a/arcade/gui/property.py +++ b/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 @@ -82,6 +85,8 @@ def remove(self, callback): @property def listeners(self) -> list[tuple[AnyListener, _ListenerType]]: """Returns a list of all listeners and type, both weak and strong.""" + # todo returning a iterator would be more efficient, but might also break + # improve ~0.01 sec return list(self._listeners.items()) @@ -221,7 +226,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 +293,25 @@ 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) diff --git a/arcade/gui/widgets/layout.py b/arcade/gui/widgets/layout.py index 2c17cf29a9..410f85159c 100644 --- a/arcade/gui/widgets/layout.py +++ b/arcade/gui/widgets/layout.py @@ -288,11 +288,11 @@ 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) @@ -300,11 +300,11 @@ def add(self, child: W, **kwargs) -> W: 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, diff --git a/tests/unit/gui/test_property.py b/tests/unit/gui/test_property.py index 8b95e37a44..86f5305d1b 100644 --- a/tests/unit/gui/test_property.py +++ b/tests/unit/gui/test_property.py @@ -144,12 +144,46 @@ 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"