diff --git a/arcade/examples/gui/6_size_hints.py b/arcade/examples/gui/6_size_hints.py index 8801eb8470..2d3959a0c3 100644 --- a/arcade/examples/gui/6_size_hints.py +++ b/arcade/examples/gui/6_size_hints.py @@ -135,7 +135,6 @@ def on_change(event: UIOnChangeEvent): content_anchor.add(UISpace(height=20)) self.ui.execute_layout() - self.ui.debug() def main(): diff --git a/arcade/gui/experimental/focus.py b/arcade/gui/experimental/focus.py index e806150120..7a394c607e 100644 --- a/arcade/gui/experimental/focus.py +++ b/arcade/gui/experimental/focus.py @@ -71,9 +71,9 @@ class UIFocusMixin(UIWidget): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - bind(self, "_debug", self.trigger_full_render) - bind(self, "_focused_widget", self.trigger_full_render) - bind(self, "_focusable_widgets", self.trigger_full_render) + bind(self, "_debug", self.trigger_full_render, weak=True) + bind(self, "_focused_widget", self.trigger_full_render, weak=True) + bind(self, "_focusable_widgets", self.trigger_full_render, weak=True) def on_event(self, event: UIEvent) -> bool | None: # pass events to children first, including controller events diff --git a/arcade/gui/experimental/scroll_area.py b/arcade/gui/experimental/scroll_area.py index fbe715cd0e..c0ae2680bc 100644 --- a/arcade/gui/experimental/scroll_area.py +++ b/arcade/gui/experimental/scroll_area.py @@ -44,12 +44,12 @@ def __init__(self, scroll_area: UIScrollArea, vertical: bool = True): self.with_border(color=arcade.uicolor.GRAY_CONCRETE) self.vertical = vertical - bind(self, "_thumb_hover", self.trigger_render) - bind(self, "_dragging", self.trigger_render) - bind(scroll_area, "scroll_x", self.trigger_full_render) - bind(scroll_area, "scroll_y", self.trigger_full_render) - bind(scroll_area, "content_height", self.trigger_full_render) - bind(scroll_area, "content_width", self.trigger_full_render) + bind(self, "_thumb_hover", self.trigger_render, weak=True) + bind(self, "_dragging", self.trigger_render, weak=True) + bind(scroll_area, "scroll_x", self.trigger_full_render, weak=True) + bind(scroll_area, "scroll_y", self.trigger_full_render, weak=True) + bind(scroll_area, "content_height", self.trigger_full_render, weak=True) + bind(scroll_area, "content_width", self.trigger_full_render, weak=True) def on_event(self, event: UIEvent) -> bool | None: # check if we are scrollable @@ -235,8 +235,8 @@ def __init__( size=canvas_size, ) - bind(self, "scroll_x", self.trigger_full_render) - bind(self, "scroll_y", self.trigger_full_render) + bind(self, "scroll_x", self.trigger_full_render, weak=True) + bind(self, "scroll_y", self.trigger_full_render, weak=True) def add(self, child: W, **kwargs) -> W: """Add a child to the widget.""" diff --git a/arcade/gui/property.py b/arcade/gui/property.py index e8ad3e8914..115a383ed4 100644 --- a/arcade/gui/property.py +++ b/arcade/gui/property.py @@ -1,8 +1,11 @@ import inspect import sys import traceback +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 @@ -18,57 +21,101 @@ AnyListener = NoArgListener | InstanceListener | InstanceValueListener | InstanceNewOldListener +class _ListenerType(Enum): + """Enum to represent the type of listener""" + + NO_ARG = 0 + INSTANCE = 1 + INSTANCE_VALUE = 2 + INSTANCE_NEW_OLD = 3 + + @staticmethod + def _detect_callback_type(callback: AnyListener) -> "_ListenerType": + """Normalizes the callback so every callback can be invoked with the same signature.""" + signature = inspect.signature(callback) + + # first detect the old *args default listener signatures + with suppress(TypeError): + signature.bind(..., ...) + return _ListenerType.INSTANCE_VALUE + + # check for the most common signature + with suppress(TypeError): + signature.bind() + return _ListenerType.NO_ARG + + # check for the other + with suppress(TypeError): + signature.bind(..., ..., ...) + return _ListenerType.INSTANCE_NEW_OLD + + with suppress(TypeError): + signature.bind(...) + return _ListenerType.INSTANCE + + raise TypeError("Callback is not callable") + + +class _CustomWeakKeyDictionary(WeakKeyDictionary): + # Instance methods are bound methods, which can not be referenced by normal `ref()` + # A normal WeakKeyDictionary, would lose the reference to the bound method immediately, + # so we need to override the __setitem__ method to use a WeakMethod instead. + + def __setitem__(self, key, value): + """Set an item in the dictionary, using a WeakMethod for bound methods.""" + if ismethod(key): + # If the key is a method, we need to use a WeakMethod + key = weakref.WeakMethod(key, self._remove) # type: ignore[assignment] + self.data[key] = value # type: ignore[assignment] + else: + super().__setitem__(key, value) + + class _Obs(Generic[P]): """ Internal holder for Property value and change listeners """ - __slots__ = ("value", "_listeners") + __slots__ = ("value", "_listeners", "_weak_listeners") def __init__(self, value: P): self.value = value # This will keep any added listener even if it is not referenced anymore # and would be garbage collected - self._listeners: dict[AnyListener, InstanceNewOldListener] = dict() + self._listeners: dict[AnyListener, _ListenerType] = dict() + # This will use weak references to the listeners, so they can be garbage collected + self._weak_listeners: WeakKeyDictionary[AnyListener, _ListenerType] = ( + _CustomWeakKeyDictionary() + ) def add( self, callback: AnyListener, + weak: bool, ): """Add a callback to the list of listeners""" - self._listeners[callback] = _Obs._normalize_callback(callback) + # Instance methods are bound methods, which can not be referenced by normal `ref()` + # if listeners would be a WeakSet, we would have to add listeners as WeakMethod + # ourselves into `WeakSet.data`. + + _listener_type = _ListenerType._detect_callback_type(callback) + if weak: + self._weak_listeners[callback] = _listener_type + else: + self._listeners[callback] = _listener_type def remove(self, callback): """Remove a callback from the list of listeners""" if callback in self._listeners: del self._listeners[callback] - @property - def listeners(self) -> list[InstanceNewOldListener]: - return list(self._listeners.values()) - - @staticmethod - def _normalize_callback(callback) -> InstanceNewOldListener: - """Normalizes the callback so every callback can be invoked with the same signature.""" - signature = inspect.signature(callback) - - with suppress(TypeError): - signature.bind(1, 1) - return lambda instance, new, old: callback(instance, new) - - with suppress(TypeError): - signature.bind(1, 1, 1) - return lambda instance, new, old: callback(instance, new, old) + elif callback in self._weak_listeners: + del self._weak_listeners[callback] - with suppress(TypeError): - signature.bind(1) - return lambda instance, new, old: callback(instance) - - with suppress(TypeError): - signature.bind() - return lambda instance, new, old: callback() - - raise TypeError("Callback is not callable") + @property + def listeners(self) -> list[tuple[AnyListener, _ListenerType]]: + """Returns a list of all listeners and type, both weak and strong.""" + return list(self._listeners.items()) + list(self._weak_listeners.items()) class Property(Generic[P]): @@ -147,9 +194,16 @@ def dispatch(self, instance, value, old_value): """ obs = self._get_obs(instance) - for listener in obs.listeners: + for listener, _listener_type in obs.listeners: try: - listener(instance, value, old_value) + if _listener_type == _ListenerType.NO_ARG: + listener() # type: ignore[call-arg] + elif _listener_type == _ListenerType.INSTANCE: + listener(instance) # type: ignore[call-arg] + elif _listener_type == _ListenerType.INSTANCE_VALUE: + listener(instance, value) # type: ignore[call-arg] + elif _listener_type == _ListenerType.INSTANCE_NEW_OLD: + listener(instance, value, old_value) # type: ignore[call-arg] except Exception: print( f"Change listener for {instance}.{self.name} = {value} raised an exception!", @@ -157,7 +211,7 @@ def dispatch(self, instance, value, old_value): ) traceback.print_exc() - def bind(self, instance, callback): + def bind(self, instance: Any, callback: AnyListener, weak: bool): """Binds a function to the change event of the property. A reference to the function will be kept. @@ -166,11 +220,7 @@ def bind(self, instance, callback): instance: The instance to bind the callback to. callback: The callback to bind. """ - obs = self._get_obs(instance) - # Instance methods are bound methods, which can not be referenced by normal `ref()` - # if listeners would be a WeakSet, we would have to add listeners as WeakMethod - # ourselves into `WeakSet.data`. - obs.add(callback) + self._get_obs(instance).add(callback, weak=weak) def unbind(self, instance, callback): """Unbinds a function from the change event of the property. @@ -200,7 +250,7 @@ def __set__(self, instance, value: P): self.set(instance, value) -def bind(instance, property: str, callback): +def bind(instance, property: str, callback: AnyListener, weak: bool = 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 @@ -224,6 +274,12 @@ class MyObject: instance: Instance owning the property property: Name of the property callback: Function to call + weak: If True, the callback will be weakly referenced. + This means that the callback will be garbage collected + if there are no other references to it. + Defaults to False. + If a property is bound to a method of an instance, it is recommended + to set this to True, so that the instance can be garbage collected. Returns: None @@ -233,7 +289,7 @@ class MyObject: if not isinstance(prop, Property): raise ValueError(f"{t.__name__}.{property} is not an arcade.gui.Property") - prop.bind(instance, callback) + prop.bind(instance, callback, weak=weak) def unbind(instance, property: str, callback): diff --git a/arcade/gui/ui_manager.py b/arcade/gui/ui_manager.py index 2fcc29f1e3..3a5707d47d 100644 --- a/arcade/gui/ui_manager.py +++ b/arcade/gui/ui_manager.py @@ -529,12 +529,10 @@ def _set_active_widget(self, widget: UIWidget | None): return if self._active_widget: - print(f"Deactivating widget {self._active_widget.__class__.__name__}") self._active_widget._active = False self._active_widget = widget if self._active_widget: - print(f"Activating widget {self._active_widget.__class__.__name__}") self._active_widget._active = True def debug(self): diff --git a/arcade/gui/widgets/__init__.py b/arcade/gui/widgets/__init__.py index 39dae93160..e0032fbbfa 100644 --- a/arcade/gui/widgets/__init__.py +++ b/arcade/gui/widgets/__init__.py @@ -1,10 +1,12 @@ from __future__ import annotations +import weakref from abc import ABC from collections.abc import Iterable from enum import IntEnum from types import EllipsisType -from typing import TYPE_CHECKING, NamedTuple, TypeVar +from typing import Any, Generic, TYPE_CHECKING, NamedTuple, TypeVar, overload +from weakref import WeakKeyDictionary from pyglet.event import EVENT_HANDLED, EVENT_UNHANDLED, EventDispatcher from pyglet.math import Vec2 @@ -31,6 +33,7 @@ from arcade.gui.ui_manager import UIManager W = TypeVar("W", bound="UIWidget") +P = TypeVar("P") class FocusMode(IntEnum): @@ -51,6 +54,51 @@ class _ChildEntry(NamedTuple): data: dict +class WeakRef(Generic[P]): + """A weak reference to a UIWidget parent, which can be used to prevent memory leaks.""" + + __slots__ = ("name", "obs") + name: str + """Attribute name of the property""" + obs: WeakKeyDictionary[Any, weakref.ref[P]] + """Weak dictionary to hold the values""" + + def __init__(self): + self.obs = WeakKeyDictionary() + + def get(self, instance: Any) -> P | None: + """Get value for owner instance""" + # If the value is not set, return None + value = self.obs.get(instance) + return value() if value else None + + def set(self, instance, value: P | None): + """Set value for owner instance""" + # Store a weak reference to the value + if value is None: + self.obs.pop(instance, None) + else: + self.obs[instance] = weakref.ref(value) + + def __set_name__(self, owner, name): + self.name = name + + @overload + def __get__(self, instance: None, instance_type) -> Self: ... + + @overload + def __get__(self, instance: Any, instance_type) -> P | None: ... + + def __get__(self, instance: Any | None, instance_type) -> Self | P | None: + """Get the value for the owner instance, or None if not set.""" + if instance is None: + return self + return self.get(instance) + + def __set__(self, instance, value: P | None): + self.set(instance, value) + + @copy_dunders_unimplemented class UIWidget(EventDispatcher, ABC): """The :class:`UIWidget` class is the base class required for creating widgets. @@ -71,6 +119,9 @@ class UIWidget(EventDispatcher, ABC): size_hint_max: max width and height in pixel """ + parent: WeakRef[UIManager | UIWidget | None] = WeakRef() + """A weak reference to the parent UIManager or UIWidget, + which does not prevent garbage collection of the parent.""" rect = Property(LBWH(0, 0, 1, 1)) visible = Property(True) focused = Property(False) @@ -113,7 +164,6 @@ def __init__( ): self._requires_render = True self.rect = LBWH(x, y, width, height) - self.parent: UIManager | UIWidget | None = None # Size hints are properties that can be used by layouts self.size_hint = size_hint @@ -126,21 +176,23 @@ def __init__( for child in children: self.add(child) - bind(self, "rect", self.trigger_full_render) - bind(self, "focused", self.trigger_full_render) + # bind properties to trigger rendering, + # use weak references to prevent memory leaks of this widget + bind(self, "rect", self.trigger_full_render, weak=True) + bind(self, "focused", self.trigger_full_render, weak=True) bind( - self, "visible", self.trigger_full_render + self, "visible", self.trigger_full_render, weak=True ) # TODO maybe trigger_parent_render would be enough - bind(self, "_children", self.trigger_render) - bind(self, "_border_width", self.trigger_render) - bind(self, "_border_color", self.trigger_render) - bind(self, "_bg_color", self.trigger_render) - bind(self, "_bg_tex", self.trigger_render) - bind(self, "_padding_top", self.trigger_render) - bind(self, "_padding_right", self.trigger_render) - bind(self, "_padding_bottom", self.trigger_render) - bind(self, "_padding_left", self.trigger_render) - bind(self, "_strong_background", self.trigger_render) + bind(self, "_children", self.trigger_render, weak=True) + bind(self, "_border_width", self.trigger_render, weak=True) + bind(self, "_border_color", self.trigger_render, weak=True) + bind(self, "_bg_color", self.trigger_render, weak=True) + bind(self, "_bg_tex", self.trigger_render, weak=True) + bind(self, "_padding_top", self.trigger_render, weak=True) + bind(self, "_padding_right", self.trigger_render, weak=True) + bind(self, "_padding_bottom", self.trigger_render, weak=True) + bind(self, "_padding_left", self.trigger_render, weak=True) + bind(self, "_strong_background", self.trigger_render, weak=True) def add(self, child: W, **kwargs) -> W: """Add a widget as a child. @@ -692,9 +744,9 @@ def __init__( self.interaction_buttons = interaction_buttons - bind(self, "pressed", self.trigger_render) - bind(self, "hovered", self.trigger_render) - bind(self, "disabled", self.trigger_render) + bind(self, "pressed", self.trigger_render, weak=True) + bind(self, "hovered", self.trigger_render, weak=True) + bind(self, "disabled", self.trigger_render, weak=True) def on_event(self, event: UIEvent) -> bool | None: """Handles mouse events and triggers on_click event if the widget is clicked. diff --git a/arcade/gui/widgets/buttons.py b/arcade/gui/widgets/buttons.py index adc0919e74..5e2d941fd1 100644 --- a/arcade/gui/widgets/buttons.py +++ b/arcade/gui/widgets/buttons.py @@ -134,7 +134,7 @@ def __init__( if texture_disabled: self._textures["disabled"] = texture_disabled - bind(self, "_textures", self.trigger_render) + bind(self, "_textures", self.trigger_render, weak=True) # prepare label with default style _style = self.get_current_style() diff --git a/arcade/gui/widgets/image.py b/arcade/gui/widgets/image.py index f732753168..a0bd9d6d4f 100644 --- a/arcade/gui/widgets/image.py +++ b/arcade/gui/widgets/image.py @@ -55,9 +55,9 @@ def __init__( height=height if height else texture.height, **kwargs, ) - bind(self, "texture", self.trigger_render) - bind(self, "alpha", self.trigger_full_render) - bind(self, "angle", self.trigger_full_render) + bind(self, "texture", self.trigger_render, weak=True) + bind(self, "alpha", self.trigger_full_render, weak=True) + bind(self, "angle", self.trigger_full_render, weak=True) @override def do_render(self, surface: Surface): diff --git a/arcade/gui/widgets/layout.py b/arcade/gui/widgets/layout.py index 386532d707..2946965e72 100644 --- a/arcade/gui/widgets/layout.py +++ b/arcade/gui/widgets/layout.py @@ -270,13 +270,13 @@ def __init__( self._size_hint_requires_update = True - bind(self, "_children", self._update_size_hints) - bind(self, "_border_width", self._update_size_hints) + bind(self, "_children", self._update_size_hints, weak=True) + bind(self, "_border_width", self._update_size_hints, weak=True) - bind(self, "_padding_left", self._update_size_hints) - bind(self, "_padding_right", self._update_size_hints) - bind(self, "_padding_top", self._update_size_hints) - bind(self, "_padding_bottom", self._update_size_hints) + bind(self, "_padding_left", self._update_size_hints, weak=True) + bind(self, "_padding_right", self._update_size_hints, weak=True) + bind(self, "_padding_top", self._update_size_hints, weak=True) + bind(self, "_padding_bottom", self._update_size_hints, weak=True) self._update_size_hints() @@ -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", self._trigger_size_hint_update) - bind(child, "rect", self._trigger_size_hint_update) - bind(child, "size_hint", self._trigger_size_hint_update) - bind(child, "size_hint_min", self._trigger_size_hint_update) - bind(child, "size_hint_max", self._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) @@ -514,13 +514,13 @@ def __init__( self.align_horizontal = align_horizontal self.align_vertical = align_vertical - bind(self, "_children", self._trigger_size_hint_update) - bind(self, "_border_width", self._trigger_size_hint_update) + bind(self, "_children", self._trigger_size_hint_update, weak=True) + bind(self, "_border_width", self._trigger_size_hint_update, weak=True) - bind(self, "_padding_left", self._trigger_size_hint_update) - bind(self, "_padding_right", self._trigger_size_hint_update) - bind(self, "_padding_top", self._trigger_size_hint_update) - bind(self, "_padding_bottom", self._trigger_size_hint_update) + bind(self, "_padding_left", self._trigger_size_hint_update, weak=True) + bind(self, "_padding_right", self._trigger_size_hint_update, weak=True) + bind(self, "_padding_top", self._trigger_size_hint_update, weak=True) + bind(self, "_padding_bottom", self._trigger_size_hint_update, weak=True) # initially update size hints # TODO is this required? @@ -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", self._trigger_size_hint_update) - bind(child, "rect", self._trigger_size_hint_update) - bind(child, "size_hint", self._trigger_size_hint_update) - bind(child, "size_hint_min", self._trigger_size_hint_update) - bind(child, "size_hint_max", self._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/arcade/gui/widgets/slider.py b/arcade/gui/widgets/slider.py index 12e632b648..edae2e6a86 100644 --- a/arcade/gui/widgets/slider.py +++ b/arcade/gui/widgets/slider.py @@ -91,11 +91,11 @@ def __init__( self._cursor_width = self.height // 3 # trigger render on value changes - bind(self, "value", self.trigger_full_render) - bind(self, "value", self._ensure_step) - bind(self, "hovered", self.trigger_render) - bind(self, "pressed", self.trigger_render) - bind(self, "disabled", self.trigger_render) + bind(self, "value", self.trigger_full_render, weak=True) + bind(self, "value", self._ensure_step, weak=True) + bind(self, "hovered", self.trigger_render, weak=True) + bind(self, "pressed", self.trigger_render, weak=True) + bind(self, "disabled", self.trigger_render, weak=True) self.register_event_type("on_change") diff --git a/arcade/gui/widgets/text.py b/arcade/gui/widgets/text.py index 3123669d58..7fb125a75b 100644 --- a/arcade/gui/widgets/text.py +++ b/arcade/gui/widgets/text.py @@ -148,14 +148,14 @@ def __init__( if height: self._label.height = int(height) - bind(self, "rect", self._update_label) + bind(self, "rect", self._update_label, weak=True) # update size hint when border or padding changes - bind(self, "_border_width", self._update_size_hint_min) - bind(self, "_padding_left", self._update_size_hint_min) - bind(self, "_padding_right", self._update_size_hint_min) - bind(self, "_padding_top", self._update_size_hint_min) - bind(self, "_padding_bottom", self._update_size_hint_min) + bind(self, "_border_width", self._update_size_hint_min, weak=True) + bind(self, "_padding_left", self._update_size_hint_min, weak=True) + bind(self, "_padding_right", self._update_size_hint_min, weak=True) + bind(self, "_padding_top", self._update_size_hint_min, weak=True) + bind(self, "_padding_bottom", self._update_size_hint_min, weak=True) self._update_size_hint_min() @@ -570,11 +570,11 @@ def __init__( self.register_event_type("on_change") - bind(self, "hovered", self._apply_style) - bind(self, "pressed", self._apply_style) - bind(self, "invalid", self._apply_style) - bind(self, "disabled", self._apply_style) - bind(self, "_active", self._on_active_changed) + bind(self, "hovered", self._apply_style, weak=True) + bind(self, "pressed", self._apply_style, weak=True) + bind(self, "invalid", self._apply_style, weak=True) + bind(self, "disabled", self._apply_style, weak=True) + bind(self, "_active", self._on_active_changed, weak=True) # initial style application self._apply_style() @@ -859,7 +859,7 @@ def __init__( multiline=multiline, ) - # bind(self, "rect", self._update_layout) + # bind(self, "rect", self._update_layout, weak=True) def fit_content(self): """Set the width and height of the text area to contain the whole text.""" diff --git a/arcade/gui/widgets/toggle.py b/arcade/gui/widgets/toggle.py index 5e1786efe2..c840912e07 100644 --- a/arcade/gui/widgets/toggle.py +++ b/arcade/gui/widgets/toggle.py @@ -82,8 +82,8 @@ def __init__( self.value = value self.register_event_type("on_change") - bind(self, "value", self.trigger_render) - bind(self, "value", self._dispatch_on_change_event) + bind(self, "value", self.trigger_render, weak=True) + bind(self, "value", self._dispatch_on_change_event, weak=True) super().__init__( x=x, diff --git a/tests/unit/gui/test_property.py b/tests/unit/gui/test_property.py index b3d25b57c6..5d0c1dc93d 100644 --- a/tests/unit/gui/test_property.py +++ b/tests/unit/gui/test_property.py @@ -213,6 +213,44 @@ def test_gc_entries_are_collected(): assert len(MyObject.name.obs) == 0 +def test_obj_collected_when_using_weak_refs_for_bound_method(): + class ObserverAndObject(Observer, MyObject): + pass + + obj = ObserverAndObject() + bind(obj, "name", obj.call, weak=True) + + # Keeps referenced objects + gc.collect() + assert len(MyObject.name.obs) == 1 + + # delete ref and trigger gc + del obj + gc.collect() + + # No leftovers + assert len(MyObject.name.obs) == 0 + + +def test_gc_entries_not_collected_by_default(): + class ObserverAndObject(Observer, MyObject): + pass + + obj = ObserverAndObject() + bind(obj, "name", obj.call, weak=False) # weak=False is the default + + # Keeps referenced objects + gc.collect() + assert len(ObserverAndObject.name.obs) == 1 + + # delete ref and trigger gc + del obj + gc.collect() + + # No leftovers + assert len(ObserverAndObject.name.obs) == 1 + + def test_gc_keeps_bound_methods(): observer = Observer() obj = MyObject() diff --git a/tests/unit/gui/test_uilabel.py b/tests/unit/gui/test_uilabel.py index a78828d166..7413deb1c9 100644 --- a/tests/unit/gui/test_uilabel.py +++ b/tests/unit/gui/test_uilabel.py @@ -79,9 +79,10 @@ def test_change_text_triggers_full_render_without_background(window): should fit the size to the text. This is not natively supported by either arcade.Text or pyglet.Label. Because text length variates between different os, we can only test boundaries, which indicate a proper implementation. """ + mock = Mock() label = UILabel(text="First Text") - label.parent = Mock() + label.parent = mock label.text = "Second Text" label.parent.trigger_render.assert_called_once() @@ -93,9 +94,10 @@ def test_change_text_triggers_render_with_background(window): should fit the size to the text. This is not natively supported by either arcade.Text or pyglet.Label. Because text length variates between different os, we can only test boundaries, which indicate a proper implementation. """ + mock = Mock() label = UILabel(text="First Text").with_background(color=Color(255, 255, 255, 255)) - label.parent = Mock() + label.parent = mock label.text = "Second Text" label.parent.trigger_render.assert_not_called() diff --git a/tests/unit/gui/test_widget_tree.py b/tests/unit/gui/test_widget_tree.py index 13b39fe78f..fee72f39c8 100644 --- a/tests/unit/gui/test_widget_tree.py +++ b/tests/unit/gui/test_widget_tree.py @@ -1,4 +1,6 @@ -from arcade.gui import UIEvent +import gc + +from arcade.gui import UIEvent, UIWidget from arcade.gui.widgets import UIDummy @@ -103,3 +105,49 @@ def test_iterate_widget_children(window): # THEN assert list(parent) == [child1, child2] + + +def test_chained_widgets_are_collected_by_gc(): + """ + Test that chained widgets are collected by garbage collector. + This is to ensure that there are no memory leaks when widgets are + added and removed in a chain. + """ + + def objs_in_memory(obj_type): + """Check if an object of a specific type is in memory.""" + return len([obj for obj in gc.get_objects() if isinstance(obj, obj_type)]) + + gc.collect() + start_count = objs_in_memory(UIWidget) + + root = UIWidget() + root.add(UIWidget()) + + # children are not collected until the parent is deleted + gc.collect() + assert objs_in_memory(UIWidget) == start_count + 2 + + del root + gc.collect() + + if objs_in_memory(UIWidget) > start_count: + print("Render object graph...") + import objgraph + + objgraph.show_chain( + objgraph.find_backref_chain( + [obj for obj in gc.get_objects() if isinstance(obj, UIWidget)][1], + objgraph.is_proper_module, + ), + # filename="chain.png", + ) + + # print("Render backrefs...") + # objgraph.show_backrefs( + # [[obj for obj in gc.get_objects() if isinstance(obj, UIWidget)][1]], + # max_depth=15, + # # filename="sample-graph.png", + # ) + + assert objs_in_memory(UIWidget) == start_count