Skip to content

Commit ecef942

Browse files
authored
Merge pull request #2748 from pythonarcade/gui/fix-memory-leak-3
gui: Fix issues with binding class methods on children
2 parents 809886e + 07431b2 commit ecef942

File tree

3 files changed

+111
-19
lines changed

3 files changed

+111
-19
lines changed

arcade/gui/property.py

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import inspect
22
import sys
33
import traceback
4+
import warnings
5+
import weakref
46
from collections.abc import Callable
57
from contextlib import contextmanager, suppress
68
from enum import Enum
9+
from inspect import ismethod
710
from typing import Any, Generic, TypeVar, cast
811
from weakref import WeakKeyDictionary, ref
912

@@ -82,6 +85,8 @@ def remove(self, callback):
8285
@property
8386
def listeners(self) -> list[tuple[AnyListener, _ListenerType]]:
8487
"""Returns a list of all listeners and type, both weak and strong."""
88+
# todo returning a iterator would be more efficient, but might also break
89+
# improve ~0.01 sec
8590
return list(self._listeners.items())
8691

8792

@@ -221,7 +226,52 @@ def __set__(self, instance, value: P):
221226
self.set(instance, value)
222227

223228

224-
def bind(instance, property: str, callback: AnyListener):
229+
class _WeakCallback:
230+
"""Wrapper for weakly referencing a callback function.
231+
232+
Which allows to bind methods of the instance itself without
233+
causing memory leaks.
234+
235+
Also supports to be stored in a dict or set, because it implements
236+
__hash__ and __eq__ methods to match the original function.
237+
"""
238+
239+
def __init__(self, func):
240+
self._func_type = _ListenerType.detect_callback_type(func) # type: ignore[assignment]
241+
self._hash = hash(func)
242+
243+
if inspect.ismethod(func):
244+
self._func = weakref.WeakMethod(func)
245+
else:
246+
self._func = weakref.ref(func)
247+
248+
def __call__(self, instance, new_value, old_value):
249+
func = self._func()
250+
if func is None:
251+
warnings.warn("WeakCallable was called without a callable object.")
252+
253+
if self._func_type == _ListenerType.NO_ARG:
254+
return func()
255+
elif self._func_type == _ListenerType.INSTANCE:
256+
return func(instance)
257+
elif self._func_type == _ListenerType.INSTANCE_VALUE:
258+
return func(instance, new_value)
259+
elif self._func_type == _ListenerType.INSTANCE_NEW_OLD:
260+
return func(instance, new_value, old_value)
261+
262+
else:
263+
raise TypeError(f"Unsupported callback type: {self._func_type}")
264+
265+
def __hash__(self):
266+
return self._hash
267+
268+
def __eq__(self, other):
269+
if ismethod(other):
270+
return self._hash == hash(other)
271+
return False
272+
273+
274+
def bind(instance, property: str, callback: AnyListener, weak=False):
225275
"""Bind a function to the change event of the property.
226276
227277
A reference to the function will be kept, so that it will be still
@@ -243,17 +293,25 @@ class MyObject:
243293
244294
Binding to a method of the Property owner itself can cause a memory leak, because the
245295
owner is strongly referenced. Instead, bind the class method, which will be invoked with
246-
the instance as first parameter.
247-
296+
the instance as first parameter. `bind(instance, "property_name", Instance.method)`.
297+
Or use the `weak` parameter to bind the method weakly
298+
bind(instance, "property_name", instance.method, weak=True)`.
248299
249300
Args:
250301
instance: Instance owning the property
251302
property: Name of the property
252303
callback: Function to call
304+
weak: If True, the callback will be weakly referenced.
305+
This is useful for methods of the instance itself to avoid memory leaks.
253306
254307
Returns:
255308
None
256309
"""
310+
311+
if weak:
312+
# If weak is True, we use a _WeakCallable to avoid strong references
313+
callback = _WeakCallback(callback) # type: ignore[assignment]
314+
257315
# TODO rename property to property_name for arcade 4.0 (just to be sure)
258316
t = type(instance)
259317
prop = getattr(t, property)

arcade/gui/widgets/layout.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -288,23 +288,23 @@ def add(self, child: W, **kwargs) -> W:
288288
child: The widget to add to the layout.
289289
"""
290290
# subscribe to child's changes, which might affect the own size hint
291-
bind(child, "_children", UIBoxLayout._trigger_size_hint_update)
292-
bind(child, "rect", UIBoxLayout._trigger_size_hint_update)
293-
bind(child, "size_hint", UIBoxLayout._trigger_size_hint_update)
294-
bind(child, "size_hint_min", UIBoxLayout._trigger_size_hint_update)
295-
bind(child, "size_hint_max", UIBoxLayout._trigger_size_hint_update)
291+
bind(child, "_children", self._trigger_size_hint_update, weak=True)
292+
bind(child, "rect", self._trigger_size_hint_update, weak=True)
293+
bind(child, "size_hint", self._trigger_size_hint_update, weak=True)
294+
bind(child, "size_hint_min", self._trigger_size_hint_update, weak=True)
295+
bind(child, "size_hint_max", self._trigger_size_hint_update, weak=True)
296296

297297
return super().add(child, **kwargs)
298298

299299
@override
300300
def remove(self, child: UIWidget):
301301
"""Remove a child from the layout."""
302302
# unsubscribe from child's changes
303-
unbind(child, "_children", UIBoxLayout._trigger_size_hint_update)
304-
unbind(child, "rect", UIBoxLayout._trigger_size_hint_update)
305-
unbind(child, "size_hint", UIBoxLayout._trigger_size_hint_update)
306-
unbind(child, "size_hint_min", UIBoxLayout._trigger_size_hint_update)
307-
unbind(child, "size_hint_max", UIBoxLayout._trigger_size_hint_update)
303+
unbind(child, "_children", self._trigger_size_hint_update)
304+
unbind(child, "rect", self._trigger_size_hint_update)
305+
unbind(child, "size_hint", self._trigger_size_hint_update)
306+
unbind(child, "size_hint_min", self._trigger_size_hint_update)
307+
unbind(child, "size_hint_max", self._trigger_size_hint_update)
308308

309309
return super().remove(child)
310310

@@ -547,11 +547,11 @@ def add(
547547
row_span: Number of rows the widget will stretch for.
548548
"""
549549
# subscribe to child's changes, which might affect the own size hint
550-
bind(child, "_children", UIGridLayout._trigger_size_hint_update)
551-
bind(child, "rect", UIGridLayout._trigger_size_hint_update)
552-
bind(child, "size_hint", UIGridLayout._trigger_size_hint_update)
553-
bind(child, "size_hint_min", UIGridLayout._trigger_size_hint_update)
554-
bind(child, "size_hint_max", UIGridLayout._trigger_size_hint_update)
550+
bind(child, "_children", self._trigger_size_hint_update, weak=True)
551+
bind(child, "rect", self._trigger_size_hint_update, weak=True)
552+
bind(child, "size_hint", self._trigger_size_hint_update, weak=True)
553+
bind(child, "size_hint_min", self._trigger_size_hint_update, weak=True)
554+
bind(child, "size_hint_max", self._trigger_size_hint_update, weak=True)
555555

556556
return super().add(
557557
child,

tests/unit/gui/test_property.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,46 @@ def test_bind_callback_with_star_args():
144144
observer.call_args = None
145145

146146

147-
def test_unbind_callback():
147+
def test_unbind_function_callback():
148+
called = False
149+
150+
def callback(*args):
151+
nonlocal called
152+
called = True
153+
154+
my_obj = MyObject()
155+
bind(my_obj, "name", callback)
156+
157+
# WHEN
158+
unbind(my_obj, "name", callback)
159+
my_obj.name = "New Name"
160+
161+
assert not called
162+
163+
164+
def test_unbind_method_callback():
148165
observer = Observer()
149166

150167
my_obj = MyObject()
151168
bind(my_obj, "name", observer.call)
152169

170+
gc.collect()
171+
172+
# WHEN
173+
unbind(my_obj, "name", observer.call)
174+
my_obj.name = "New Name"
175+
176+
assert not observer.called
177+
178+
179+
def test_unbind_weak_method_callback():
180+
observer = Observer()
181+
182+
my_obj = MyObject()
183+
bind(my_obj, "name", observer.call, weak=True)
184+
185+
gc.collect()
186+
153187
# WHEN
154188
unbind(my_obj, "name", observer.call)
155189
my_obj.name = "New Name"

0 commit comments

Comments
 (0)