fix(dispatcher): remove sender verification from SafeSlot

Keep receiver lifetime checks in dispatcher invalidation and relay delivery.
Deliver subscription context through metadata so waveform async updates
and log ingestion work without QObject.sender(). Reject stale scan updates.

Add regression coverage for queued delivery, subscription context isolation,
and waveform and logpanel updates through the dispatcher relay.
This commit is contained in:
2026-10-05 15:52:57 +02:00
committed by Jan Wyzula
parent bd0a231863
commit efc65bdf07
11 changed files with 162 additions and 85 deletions
+6 -2
View File
@@ -85,7 +85,8 @@ class QtThreadSafeCallback(QObject):
Args:
cb (Callable): The callback function to be wrapped.
cb_info (dict, optional): Additional information about the callback. Defaults to None.
cb_info (dict, optional): Subscription context delivered as ``metadata["cb_info"]``.
Defaults to None.
owner (object, optional): Lifetime anchor for non-method callables. Ignored for
bound methods (their ``__self__`` is the owner). Defaults to None.
"""
@@ -147,6 +148,8 @@ class QtThreadSafeCallback(QObject):
# the receiver alive while its callback runs.
callback = self.cb_ref()
if callback is not None:
if self.cb_info is not None:
metadata = {**metadata, "cb_info": dict(self.cb_info)}
callback(msg_content, metadata)
def __call__(self, msg_content, metadata):
@@ -259,7 +262,8 @@ class BECDispatcher:
slot (Callable): A slot method/function that accepts two inputs: content and metadata of
the corresponding pub/sub message
topics EndpointInfo | str | list[EndpointInfo] | list[str]: A topic or list of topics that can typically be acquired via bec_lib.MessageEndpoints
cb_info (dict | None): A dictionary containing information about the callback. Defaults to None.
cb_info (dict | None): Subscription context delivered as ``metadata["cb_info"]``.
Defaults to None. Callbacks must use this context rather than ``QObject.sender()``.
owner (object | None): Lifetime anchor for non-method callables (lambdas, partials,
module functions): the subscription is released when the owner is cleaned up or
destroyed. Bound methods already carry their owner and ignore this. Defaults to None.
-17
View File
@@ -247,9 +247,6 @@ def SafeSlot(*slot_args, **slot_kwargs): # pylint: disable=invalid-name
'popup_error' keyword argument can be passed with boolean value if a dialog should pop up,
otherwise error display is left to the original exception hook
'verify_sender' keyword argument can be passed with boolean value if the sender should be verified
before executing the slot. If True, the slot will only execute if the sender is a QObject. This is
useful to prevent function calls from already deleted objects.
'raise_error' keyword argument can be passed with boolean value if the error should be raised
after the error is displayed. This is useful to propagate the error to the caller but should be used
with great care to avoid segfaults.
@@ -261,7 +258,6 @@ def SafeSlot(*slot_args, **slot_kwargs): # pylint: disable=invalid-name
"""
_slot_params = {
"popup_error": bool(slot_kwargs.pop("popup_error", False)),
"verify_sender": bool(slot_kwargs.pop("verify_sender", False)),
"raise_error": bool(slot_kwargs.pop("raise_error", RAISE_ERROR_DEFAULT)),
}
@@ -275,19 +271,6 @@ def SafeSlot(*slot_args, **slot_kwargs): # pylint: disable=invalid-name
# only, not permanently mutate the decorator's defaults.
call_params = {**_slot_params, **_override_slot_params}
try:
if not call_params["verify_sender"] or len(args) == 0:
return method(*args, **kwargs)
_instance = args[0]
if not isinstance(_instance, QObject):
return method(*args, **kwargs)
sender = _instance.sender()
if sender is None:
logger.info(
f"Sender is None for {method.__module__}.{method.__qualname__}, "
"skipping method call."
)
return
return method(*args, **kwargs)
except Exception:
+1 -1
View File
@@ -671,7 +671,7 @@ class Heatmap(ImageBase):
QTimer.singleShot(100, self.update_plot)
QTimer.singleShot(300, self.update_plot)
@SafeSlot(verify_sender=True)
@SafeSlot()
def update_plot(self, _=None) -> None:
"""
Update the plot with the current data.
@@ -332,7 +332,7 @@ class CurveRow(QTreeWidgetItem):
self._update_composite_button_text()
dialog.deleteLater()
@SafeSlot(str, verify_sender=True)
@SafeSlot(str)
def _on_color_changed(self, new_color: str):
"""
Update configuration when the color button emits a change.
@@ -1822,7 +1822,7 @@ class Waveform(PlotBase):
)
logger.info(f"Setup async curve {name} (signal '{signal}', endpoint '{endpoint_str}')")
@SafeSlot(dict, dict, verify_sender=True)
@SafeSlot(dict, dict)
def on_async_readback(self, msg, metadata):
"""
Get async data readback. This code needs to be fast, therefor we try
@@ -1839,15 +1839,16 @@ class Waveform(PlotBase):
Args:
msg(dict): Message with the async data.
metadata(dict): Metadata of the message.
metadata(dict): Message metadata, including the subscription's scan ID in ``cb_info``.
"""
sender = self.sender()
if not hasattr(sender, "cb_info"):
logger.info(f"Sender {sender} has no cb_info.")
cb_info = metadata.get("cb_info")
if not isinstance(cb_info, dict):
logger.info("Async readback has no subscription context.")
return
scan_id = sender.cb_info.get("scan_id", None)
scan_id = cb_info.get("scan_id")
if scan_id != self.scan_id:
logger.info("Scan ID mismatch, ignoring async readback.")
return
instruction = metadata.get("async_update", {}).get("type")
if instruction not in ["add", "add_slice", "replace"]:
@@ -238,7 +238,7 @@ class BecLogsQueue(BECConnector, QObject):
BecLogsQueue._instance = None
self.deleteLater()
@SafeSlot(verify_sender=True)
@SafeSlot()
def _process_incoming_log_msg(self, msg: dict, _metadata: dict):
try:
_msg = LogMessage(**msg)
@@ -248,7 +248,7 @@ class BecLogsQueue(BECConnector, QObject):
return
logger.warning(f"Error in LogPanel incoming message callback: {e}")
@SafeSlot(verify_sender=True)
@SafeSlot()
def _proc_update(self):
if self._paused:
self.buffered.emit(len(self._incoming))
+43
View File
@@ -368,6 +368,49 @@ def test_qthreadsafe_callback_delivers_live_receiver_on_gui_thread(qapp):
assert calls == [({"value": 1}, {"source": "worker"}, qapp.thread())]
def test_qthreadsafe_callback_delivers_subscription_context_to_qobject(qapp):
"""QObject slots receive subscription context without relying on a Qt sender."""
from bec_widgets.utils.error_popups import SafeSlot
calls = []
class Receiver(QObject):
@SafeSlot(dict, dict)
def receive(self, content, metadata):
calls.append((content, metadata, QThread.currentThread()))
receiver = Receiver()
wrapper = QtThreadSafeCallback(receiver.receive, cb_info={"scan_id": "scan-1"})
_queue_callback(wrapper)
assert calls == []
qapp.processEvents()
assert calls == [
({"value": 1}, {"source": "worker", "cb_info": {"scan_id": "scan-1"}}, qapp.thread())
]
def test_qthreadsafe_callback_subscription_context_is_not_shared():
"""A callback cannot mutate shared message metadata or another subscription's context."""
received = []
def callback(_content, metadata):
received.append(metadata["cb_info"]["scan_id"])
metadata["cb_info"]["scan_id"] = "changed"
metadata["source"] = "changed"
first = QtThreadSafeCallback(callback, cb_info={"scan_id": "scan-1"})
second = QtThreadSafeCallback(callback, cb_info={"scan_id": "scan-2"})
metadata = {"source": "worker", "cb_info": {"scan_id": "message-context"}}
first({}, metadata)
second({}, metadata)
assert received == ["scan-1", "scan-2"]
assert metadata == {"source": "worker", "cb_info": {"scan_id": "message-context"}}
assert first.cb_info == {"scan_id": "scan-1"}
assert second.cb_info == {"scan_id": "scan-2"}
@pytest.mark.parametrize(
"disconnect", ["disconnect_owner", "disconnect_slot", "disconnect_topics", "disconnect_all"]
)
+12 -27
View File
@@ -40,18 +40,12 @@ class TestSafeSlotClass(QObject):
def __init__(self, parent=None, signal_obj: TestSafeSlotEmitter | None = None):
super().__init__(parent)
assert signal_obj is not None, "Signal object must be provided"
signal_obj.test_signal.connect(self.method_without_sender_verification)
signal_obj.test_signal.connect(self.method_with_sender_verification)
self._method_without_verification_called = False
self._method_with_verification_called = False
signal_obj.test_signal.connect(self.receive)
self.calls = 0
@SafeSlot()
def method_without_sender_verification(self):
self._method_without_verification_called = True
@SafeSlot(verify_sender=True)
def method_with_sender_verification(self):
self._method_with_verification_called = True
def receive(self):
self.calls += 1
@pytest.fixture
@@ -174,25 +168,16 @@ def test_safe_property_setter_error(mock_exec, mock_log_error, qtbot, global_pop
@pytest.mark.timeout(100)
def test_safe_slot_emit(qtbot):
"""
Test that the signal is emitted correctly.
"""
@pytest.mark.parametrize("delivery", ["signal", "direct"])
def test_safe_slot_emit(qtbot, delivery):
"""SafeSlot executes both signal deliveries and ordinary Python calls."""
signal_obj = TestSafeSlotEmitter()
test_obj = TestSafeSlotClass(signal_obj=signal_obj)
signal_obj.test_signal.emit()
qtbot.waitUntil(lambda: test_obj._method_without_verification_called, timeout=1000)
qtbot.waitUntil(lambda: test_obj._method_with_verification_called, timeout=1000)
test_obj.deleteLater()
test_obj = TestSafeSlotClass(signal_obj=signal_obj)
test_obj.method_without_sender_verification()
test_obj.method_with_sender_verification()
assert test_obj._method_without_verification_called is True
assert test_obj._method_with_verification_called is False
if delivery == "signal":
signal_obj.test_signal.emit()
else:
test_obj.receive()
qtbot.waitUntil(lambda: test_obj.calls == 1, timeout=1000)
test_obj.deleteLater()
signal_obj.deleteLater()
+4 -4
View File
@@ -357,7 +357,7 @@ def test_heatmap_update_plot_no_scan_item(heatmap_widget):
color_map="viridis",
)
with mock.patch.object(heatmap_widget.main_image, "setImage") as mock_set_image:
heatmap_widget.update_plot(_override_slot_params={"verify_sender": False})
heatmap_widget.update_plot()
mock_set_image.assert_not_called()
@@ -383,7 +383,7 @@ def test_heatmap_update_plot(heatmap_widget):
request_inputs={"arg_bundle": ["samx", -5, 5, 10, "samy", -5, 5, 10], "kwargs": {}},
)
with mock.patch.object(heatmap_widget.main_image, "setImage") as mock_set_image:
heatmap_widget.update_plot(_override_slot_params={"verify_sender": False})
heatmap_widget.update_plot()
img = mock_set_image.mock_calls[0].args[0]
assert img.shape == (10, 10)
@@ -399,7 +399,7 @@ def test_heatmap_update_plot_without_status_message(heatmap_widget):
heatmap_widget.scan_item = create_dummy_scan_item()
heatmap_widget.scan_item.status_message = None
with mock.patch.object(heatmap_widget.main_image, "setImage") as mock_set_image:
heatmap_widget.update_plot(_override_slot_params={"verify_sender": False})
heatmap_widget.update_plot()
mock_set_image.assert_not_called()
@@ -422,7 +422,7 @@ def test_heatmap_update_plot_no_img_data(heatmap_widget):
)
with mock.patch.object(heatmap_widget, "get_image_data", return_value=None):
with mock.patch.object(heatmap_widget.main_image, "setImage") as mock_set_image:
heatmap_widget.update_plot(_override_slot_params={"verify_sender": False})
heatmap_widget.update_plot()
mock_set_image.assert_not_called()
+24 -5
View File
@@ -133,8 +133,7 @@ def test_log_panel_update(qtbot, log_panel: LogPanel):
},
)
)
# emit through the timer: _proc_update verifies its sender and skips plain calls
log_panel._model.log_queue._update_timer.timeout.emit()
log_panel._model.log_queue._proc_update()
qtbot.waitUntil(lambda: log_panel._model.rowCount() == 4, timeout=500)
@@ -154,12 +153,32 @@ def make_log_msg(i: int, log_type: str = "info", service: str = "ScanServer") ->
)
def test_log_panel_ingests_queued_dispatcher_message(qtbot, log_panel: LogPanel):
"""Log ingestion works through the relay without a Qt sender context."""
import threading
queue = log_panel._model.log_queue
queue._update_timer.stop()
wrapper = next(
slot
for slot in queue.bec_dispatcher._registered_slots.values()
if slot.cb == queue._process_incoming_log_msg
)
thread = threading.Thread(target=wrapper, args=(make_log_msg(1).model_dump(), {}))
thread.start()
thread.join(timeout=5)
assert not thread.is_alive()
assert len(queue._incoming) == 0
qtbot.waitUntil(lambda: len(queue._incoming) == 1)
queue._proc_update()
assert log_panel._model.rowCount() == 4
def _feed(log_panel: LogPanel, messages: list[LogMessage]):
queue = log_panel._model.log_queue
queue._incoming.extend(messages)
# emit through the timer so _proc_update's verify_sender check passes (a plain
# method call is skipped with "Sender is None")
queue._update_timer.timeout.emit()
queue._proc_update()
def _patched_const(monkeypatch, **overrides):
+62 -20
View File
@@ -1,6 +1,7 @@
from __future__ import annotations
import json
import threading
from types import SimpleNamespace
from unittest import mock
from unittest.mock import MagicMock
@@ -1083,15 +1084,12 @@ def test_on_async_readback_add_update(qtbot, mocked_client):
############# Test add ################
msg = {"signals": {"async_device": {"value": [100, 200], "timestamp": [1001, 1002]}}}
metadata = {"async_update": {"max_shape": [None], "type": "add"}}
metadata = {
"cb_info": {"scan_id": wf.scan_id},
"async_update": {"max_shape": [None], "type": "add"},
}
cb_info_ret = {"scan_id": wf.scan_id}
def ret_sender():
return SimpleNamespace(cb_info={"scan_id": wf.scan_id})
with mock.patch.object(wf, "sender", side_effect=ret_sender):
wf.on_async_readback(msg, metadata, _override_slot_params={"verify_sender": False})
wf.on_async_readback(msg, metadata)
x_data, y_data = c.get_data()
assert len(x_data) == 5
@@ -1102,9 +1100,11 @@ def test_on_async_readback_add_update(qtbot, mocked_client):
# instruction='replace'
msg2 = {"signals": {"async_device": {"value": [999], "timestamp": [555]}}}
metadata2 = {"async_update": {"max_shape": [None], "type": "replace"}}
with mock.patch.object(wf, "sender", side_effect=ret_sender):
wf.on_async_readback(msg2, metadata2, _override_slot_params={"verify_sender": False})
metadata2 = {
"cb_info": {"scan_id": wf.scan_id},
"async_update": {"max_shape": [None], "type": "replace"},
}
wf.on_async_readback(msg2, metadata2)
x_data2, y_data2 = c.get_data()
np.testing.assert_array_equal(x_data2, [0])
@@ -1117,10 +1117,10 @@ def test_on_async_readback_add_update(qtbot, mocked_client):
for ii in range(10):
msg = {"signals": {"async_device": {"value": [100], "timestamp": [1001]}}}
metadata = {
"async_update": {"max_shape": [None, waveform_shape], "index": 0, "type": "add_slice"}
"cb_info": {"scan_id": wf.scan_id},
"async_update": {"max_shape": [None, waveform_shape], "index": 0, "type": "add_slice"},
}
with mock.patch.object(wf, "sender", side_effect=ret_sender):
wf.on_async_readback(msg, metadata, _override_slot_params={"verify_sender": False})
wf.on_async_readback(msg, metadata)
# Old data should be deleted since the slice_index did not match
x_data, y_data = c.get_data()
@@ -1145,10 +1145,10 @@ def test_on_async_readback_add_update(qtbot, mocked_client):
}
}
metadata = {
"async_update": {"max_shape": [None, waveform_shape], "index": 0, "type": "add_slice"}
"cb_info": {"scan_id": wf.scan_id},
"async_update": {"max_shape": [None, waveform_shape], "index": 0, "type": "add_slice"},
}
with mock.patch.object(wf, "sender", side_effect=ret_sender):
wf.on_async_readback(msg, metadata, _override_slot_params={"verify_sender": False})
wf.on_async_readback(msg, metadata)
x_data, y_data = c.get_data()
assert len(y_data) == waveform_shape
assert len(x_data) == waveform_shape
@@ -1168,9 +1168,8 @@ def test_on_async_readback_add_update(qtbot, mocked_client):
}
}
}
metadata = {"async_update": {"type": "replace"}}
with mock.patch.object(wf, "sender", side_effect=ret_sender):
wf.on_async_readback(msg, metadata, _override_slot_params={"verify_sender": False})
metadata = {"cb_info": {"scan_id": wf.scan_id}, "async_update": {"type": "replace"}}
wf.on_async_readback(msg, metadata)
x_data, y_data = c.get_data()
assert np.array_equal(y_data, np.array(range(waveform_shape)))
@@ -1180,6 +1179,49 @@ def test_on_async_readback_add_update(qtbot, mocked_client):
assert len(y_displayed) == waveform_shape
def test_on_async_readback_through_dispatcher(qtbot, mocked_client):
"""Queued relay delivery updates async curves without a Qt sender context."""
wf = create_widget(qtbot, Waveform, client=mocked_client)
wf.scan_id = "async-test"
wf.scan_item = create_dummy_scan_item(scan_id=wf.scan_id)
wf.x_axis_mode["name"] = "index"
curve = wf.plot(arg1="async_device")
qtbot.wait(50)
curve.setData([0], [0])
wrapper = next(
slot
for slot in wf.bec_dispatcher._registered_slots.values()
if slot.cb == wf.on_async_readback and slot.cb_info == {"scan_id": wf.scan_id}
)
msg = {"signals": {"async_device": {"value": [123, 456]}}}
metadata = {"async_update": {"type": "replace"}}
thread = threading.Thread(target=wrapper, args=(msg, metadata))
thread.start()
thread.join(timeout=5)
assert not thread.is_alive()
np.testing.assert_array_equal(curve.get_data()[1], [0])
qtbot.waitUntil(lambda: len(curve.get_data()[1]) == 2)
np.testing.assert_array_equal(curve.get_data()[1], [123, 456])
@pytest.mark.parametrize("cb_info", [None, {}, {"scan_id": "old-scan"}])
def test_on_async_readback_ignores_missing_or_stale_context(qtbot, mocked_client, cb_info):
"""Missing subscription context and messages for an old scan do not update curves."""
wf = create_widget(qtbot, Waveform, client=mocked_client)
wf.scan_id = "current-scan"
wf.scan_item = create_dummy_scan_item(scan_id=wf.scan_id)
curve = wf.plot(arg1="async_device")
msg = {"signals": {"async_device": {"value": [123]}}}
metadata = {"async_update": {"type": "replace"}}
if cb_info is not None:
metadata["cb_info"] = cb_info
with mock.patch.object(curve, "setData") as set_data:
wf.on_async_readback(msg, metadata)
set_data.assert_not_called()
def test_get_x_data(qtbot, mocked_client, monkeypatch):
"""
Test _get_x_data logic for multiple modes: 'timestamp', 'index', 'custom', 'auto'.