mirror of
https://github.com/bec-project/bec_widgets.git
synced 2026-10-31 16:41:27 +01:00
fix(theme): invalidate widget theme connections during cleanup
This commit is contained in:
1 parent
ed145cc6d6
commit
a03f868af7
4 files changed
+108
-28
No files matched your search
@@ -111,8 +111,9 @@ class BECWidget(BECConnector):
|
||||
def _connect_to_theme_change(self):
|
||||
"""Connect to the theme change signal."""
|
||||
qapp = QApplication.instance()
|
||||
self._theme_connection = None
|
||||
if hasattr(qapp, "theme"):
|
||||
SafeConnect(self, qapp.theme.theme_changed, self._update_theme)
|
||||
self._theme_connection = SafeConnect(self, qapp.theme.theme_changed, self._update_theme)
|
||||
|
||||
@SafeSlot(str)
|
||||
@SafeSlot()
|
||||
@@ -402,6 +403,9 @@ class BECWidget(BECConnector):
|
||||
|
||||
def cleanup(self):
|
||||
"""Cleanup the widget."""
|
||||
theme_connection = getattr(self, "_theme_connection", None)
|
||||
if theme_connection is not None:
|
||||
theme_connection.invalidate()
|
||||
with RPCRegister.delayed_broadcast():
|
||||
# All widgets need to call super().cleanup() in their cleanup method
|
||||
logger.info(f"Registry cleanup for widget {self.__class__.__name__}")
|
||||
|
||||
@@ -192,13 +192,55 @@ def SafeProperty(
|
||||
return decorator
|
||||
|
||||
|
||||
class _SafeConnection:
|
||||
"""Weak receiver wrapper with explicit invalidation for pending deliveries."""
|
||||
|
||||
def __init__(self, instance, signal, slot):
|
||||
self._active = True
|
||||
self._connected = True
|
||||
self._instance = safe_ref(instance)
|
||||
self._slot = safe_ref(slot)
|
||||
self._signal = signal
|
||||
|
||||
def invalidate(self, *_args) -> None:
|
||||
"""Block deliveries without touching Qt during receiver destruction."""
|
||||
self._active = False
|
||||
|
||||
def _disconnect(self) -> None:
|
||||
if not self._connected:
|
||||
return
|
||||
self._connected = False
|
||||
try:
|
||||
self._signal.disconnect(self)
|
||||
except (RuntimeError, TypeError):
|
||||
pass
|
||||
|
||||
def __call__(self, *args):
|
||||
if not self._active:
|
||||
self._disconnect()
|
||||
return
|
||||
instance = self._instance()
|
||||
slot = self._slot()
|
||||
if (
|
||||
instance is None
|
||||
or slot is None
|
||||
or getattr(instance, "_destroyed", False)
|
||||
or not shiboken6.isValid(instance)
|
||||
):
|
||||
self.invalidate()
|
||||
self._disconnect()
|
||||
return
|
||||
slot(*args)
|
||||
|
||||
|
||||
def SafeConnect(instance, signal, slot): # pylint: disable=invalid-name
|
||||
"""
|
||||
Method to safely handle Qt signal-slot connections. The python object is only forwarded
|
||||
as a weak reference to avoid stale objects. Once the instance or slot has been garbage
|
||||
collected, marked as destroyed, or its C++ object deleted, the connection removes itself
|
||||
from the signal on the next emission, so long-lived signals (e.g. theme_changed) do not
|
||||
accumulate dead wrappers.
|
||||
accumulate dead wrappers. Receiver destruction first marks the wrapper inactive without
|
||||
disconnecting Qt signals in destructor context.
|
||||
|
||||
Args:
|
||||
instance: The instance to connect.
|
||||
@@ -206,38 +248,17 @@ def SafeConnect(instance, signal, slot): # pylint: disable=invalid-name
|
||||
slot: The slot to connect.
|
||||
|
||||
Returns:
|
||||
Callable: The wrapper connected to the signal. It can be passed to
|
||||
Callable: The wrapper connected to the signal. Its ``invalidate()`` method blocks
|
||||
pending deliveries at cleanup start. It can also be passed to
|
||||
``signal.disconnect`` for explicit early disconnection.
|
||||
|
||||
Example:
|
||||
>>> SafeConnect(self, qapp.theme.theme_changed, self._update_theme)
|
||||
|
||||
"""
|
||||
weak_instance = safe_ref(instance)
|
||||
weak_slot = safe_ref(slot)
|
||||
|
||||
def safe_slot(*connect_args):
|
||||
instance_ = weak_instance()
|
||||
slot_func = weak_slot()
|
||||
|
||||
# Instance or slot garbage collected, instance cleaned up, or C++ side
|
||||
# deleted: remove this stale connection from the signal.
|
||||
if (
|
||||
instance_ is None
|
||||
or slot_func is None
|
||||
or getattr(instance_, "_destroyed", False)
|
||||
or not shiboken6.isValid(instance_)
|
||||
):
|
||||
try:
|
||||
signal.disconnect(safe_slot)
|
||||
except (RuntimeError, TypeError):
|
||||
# Signal owner already gone or connection already removed.
|
||||
pass
|
||||
return
|
||||
|
||||
slot_func(*connect_args)
|
||||
|
||||
safe_slot = _SafeConnection(instance, signal, slot)
|
||||
signal.connect(safe_slot)
|
||||
instance.destroyed.connect(safe_slot.invalidate)
|
||||
return safe_slot
|
||||
|
||||
|
||||
|
||||
@@ -45,6 +45,21 @@ def test_close_runs_cleanup_once(qtbot, mocked_client):
|
||||
_flush_deferred_deletes(qtbot)
|
||||
|
||||
|
||||
def test_cleanup_invalidates_theme_subscription_while_widget_is_valid(qtbot, mocked_client):
|
||||
widget = LifecycleWidget(client=mocked_client)
|
||||
connection = widget._theme_connection
|
||||
assert connection._active
|
||||
|
||||
widget.cleanup()
|
||||
assert shiboken6.isValid(widget)
|
||||
assert not widget._destroyed
|
||||
assert not connection._active
|
||||
|
||||
widget.close()
|
||||
widget.deleteLater()
|
||||
_flush_deferred_deletes(qtbot)
|
||||
|
||||
|
||||
def test_delete_later_without_close_runs_cleanup(qtbot, mocked_client):
|
||||
widget = LifecycleWidget(client=mocked_client)
|
||||
gui_id = widget.gui_id
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
import shiboken6
|
||||
from bec_lib.logger import bec_logger
|
||||
from qtpy.QtCore import QObject, Signal
|
||||
from qtpy.QtCore import QObject, Qt, Signal
|
||||
from qtpy.QtWidgets import QMessageBox
|
||||
|
||||
from bec_widgets.utils.error_popups import ErrorPopupUtility, ExampleWidget, SafeProperty, SafeSlot
|
||||
@@ -243,6 +244,45 @@ def test_safe_connect_removes_stale_connection(qtbot):
|
||||
emitter.deleteLater()
|
||||
|
||||
|
||||
def test_safe_connect_invalidated_before_queued_delivery(qtbot):
|
||||
"""An inactive subscription must not deliver an already queued signal."""
|
||||
from bec_widgets.utils.error_popups import SafeConnect
|
||||
|
||||
emitter = _ArgSignalEmitter()
|
||||
receiver = _SafeConnectReceiver()
|
||||
wrapper = SafeConnect(receiver, emitter.str_signal, receiver.on_value)
|
||||
emitter.str_signal.disconnect(wrapper)
|
||||
emitter.str_signal.connect(wrapper, Qt.ConnectionType.QueuedConnection)
|
||||
|
||||
emitter.str_signal.emit("queued")
|
||||
assert receiver.calls == []
|
||||
wrapper.invalidate()
|
||||
qtbot.wait(10)
|
||||
assert shiboken6.isValid(receiver)
|
||||
assert receiver.calls == []
|
||||
# Also cover direct calls retained by code outside the Qt connection.
|
||||
wrapper("direct")
|
||||
assert receiver.calls == []
|
||||
receiver.deleteLater()
|
||||
emitter.deleteLater()
|
||||
|
||||
|
||||
def test_safe_connect_destruction_only_invalidates():
|
||||
"""Destruction blocks delivery without disconnecting signals in the destructor."""
|
||||
from bec_widgets.utils.error_popups import SafeConnect
|
||||
|
||||
emitter = _ArgSignalEmitter()
|
||||
receiver = _SafeConnectReceiver()
|
||||
wrapper = SafeConnect(receiver, emitter.str_signal, receiver.on_value)
|
||||
with patch.object(wrapper, "_disconnect") as disconnect:
|
||||
shiboken6.delete(receiver)
|
||||
assert not wrapper._active
|
||||
disconnect.assert_not_called()
|
||||
emitter.str_signal.emit("after-death")
|
||||
assert not wrapper._connected
|
||||
shiboken6.delete(emitter)
|
||||
|
||||
|
||||
def test_safe_slot_override_params_apply_to_single_call():
|
||||
"""Regression test for BW-004: _override_slot_params must not permanently
|
||||
mutate the decorator's defaults."""
|
||||
|
||||
Reference in new issue
Block a user