fix(rpc): weak, deduplicated registry callbacks with owner-safe lifecycle

bec_widgets/applications/launch_window.py

@@ -282,7 +282,7 @@ class LaunchWindow(BECMainWindow):
         self._update_theme()
 
         self.register = RPCRegister()
-        self.register.callbacks.append(self._turn_off_the_lights)
+        self.register.add_callback(self._turn_off_the_lights)
         self.register.broadcast()
 
         if launch_gui_class and launch_gui_id:
@@ -722,6 +722,14 @@ class LaunchWindow(BECMainWindow):
         if self.app:
             self.app.setQuitOnLastWindowClosed(True)  # type: ignore
 
+    def cleanup(self):
+        """
+        Deregister the turn-off-the-lights callback before teardown so later
+        registry broadcasts never call into a dying launcher.
+        """
+        self.register.remove_callback(self._turn_off_the_lights)
+        super().cleanup()
+
     def closeEvent(self, event):
         """
         Close the launcher window.

bec_widgets/utils/rpc_register.py

@@ -7,6 +7,7 @@ from weakref import WeakValueDictionary
 
 import shiboken6 as shb
 from bec_lib.logger import bec_logger
+from louie.saferef import safe_ref
 from qtpy.QtCore import QObject
 
 if TYPE_CHECKING:  # pragma: no cover
@@ -147,14 +148,33 @@ class RPCRegister:
 
     def broadcast(self):
         """
-        Broadcast the update to all the callbacks.
+        Broadcast the update to all the callbacks. Callbacks whose owners have
+        been garbage collected — or whose owning QObject's C++ side has been
+        destroyed while the Python wrapper is still referenced — are pruned
+        instead of being called.
         """
 
         if self._skip_broadcast:
             return
         connections = self.list_all_connections()
-        for callback in self.callbacks:
+        dead_refs = []
+        for callback_ref in list(self.callbacks):
+            callback = callback_ref()
+            if callback is None:
+                dead_refs.append(callback_ref)
+                continue
+            owner = getattr(callback, "__self__", None)
+            if isinstance(owner, QObject) and not shb.isValid(owner):
+                # Bound method of a destroyed widget (Python wrapper still alive,
+                # e.g. during shutdown): calling it would raise on any Qt access.
+                dead_refs.append(callback_ref)
+                continue
             callback(connections)
+        for ref in dead_refs:
+            try:
+                self.callbacks.remove(ref)
+            except ValueError:
+                pass
 
     def object_is_registered(self, obj: BECConnector) -> bool:
         """
@@ -172,22 +192,30 @@ class RPCRegister:
         """
         Add a callback that will be called whenever the registry is updated.
 
+        Callbacks are stored as weak references (safe for bound methods): the
+        register never keeps a callback's owner alive. Registering the same
+        callback twice is a no-op, so broadcasts are delivered exactly once
+        per registered callback.
+
         Args:
             callback(Callable[[dict], None]): The callback to be added. It should accept a dictionary of all the
             registered RPC objects as an argument.
         """
-        self.callbacks.append(callback)
+        callback_ref = safe_ref(callback)
+        if callback_ref not in self.callbacks:
+            self.callbacks.append(callback_ref)
 
     def remove_callback(self, callback: Callable[[dict], None]):
         """
         Remove a previously added registry-update callback. Removing a callback
-        that is not registered is a no-op.
+        that is not registered (or removing it twice) is a no-op.
 
         Args:
             callback(Callable[[dict], None]): The callback to be removed.
         """
+        callback_ref = safe_ref(callback)
         try:
-            self.callbacks.remove(callback)
+            self.callbacks.remove(callback_ref)
         except ValueError:
             pass
 

tests/unit_tests/test_bec_connector.py

@@ -158,9 +158,12 @@ def test_bec_widget_cleanup_broadcasts_after_children_are_unregistered(mocked_cl
     qtbot.addWidget(parent)
 
     observed_connections = []
-    parent.rpc_register.callbacks.append(
-        lambda connections: observed_connections.append(set(connections))
-    )
+
+    # Keep a strong reference: registry callbacks are weakly referenced.
+    def _observe_connections(connections):
+        observed_connections.append(set(connections))
+
+    parent.rpc_register.add_callback(_observe_connections)
 
     parent.close()
 

tests/unit_tests/test_launch_window.py

@@ -266,3 +266,14 @@ def test_main_label_point_size_uniform(bec_launch_window):
     """
     point_sizes = {tile.main_label.font().pointSize() for tile in bec_launch_window.tiles.values()}
     assert len(point_sizes) == 1, f"Non-uniform main-label point sizes: {point_sizes}"
+
+
+def test_launch_window_cleanup_deregisters_registry_callback(bec_launch_window):
+    """The turn-off-the-lights callback must not survive the launcher's cleanup:
+    a later registry broadcast would call into a dying window (shutdown scenario)."""
+    from louie.saferef import safe_ref
+
+    reg = bec_launch_window.register
+    assert safe_ref(bec_launch_window._turn_off_the_lights) in reg.callbacks
+    bec_launch_window.cleanup()
+    assert safe_ref(bec_launch_window._turn_off_the_lights) not in reg.callbacks

tests/unit_tests/test_rpc_register.py

@@ -50,3 +50,83 @@ def test_reset_singleton(rpc_register):
 
     assert len(all_connections) == 0
     assert all_connections == {}
+
+
+class _CallbackOwner:
+    """Owner of a bound-method registry callback for lifecycle tests."""
+
+    def __init__(self):
+        self.received = []
+
+    def on_update(self, connections):
+        self.received.append(dict(connections))
+
+
+def test_register_callback_receives_broadcast(rpc_register):
+    owner = _CallbackOwner()
+    rpc_register.add_callback(owner.on_update)
+
+    rpc_register.broadcast()
+    assert len(owner.received) == 1
+
+
+def test_duplicate_callback_registration_is_deduplicated(rpc_register):
+    """Regression test for BW-009: registering the same bound method twice
+    must deliver each broadcast exactly once."""
+    owner = _CallbackOwner()
+    callbacks_before = len(rpc_register.callbacks)
+
+    rpc_register.add_callback(owner.on_update)
+    rpc_register.add_callback(owner.on_update)
+    assert len(rpc_register.callbacks) == callbacks_before + 1
+
+    rpc_register.broadcast()
+    assert len(owner.received) == 1
+
+
+def test_remove_callback_is_idempotent(rpc_register):
+    owner = _CallbackOwner()
+    callbacks_before = len(rpc_register.callbacks)
+
+    rpc_register.add_callback(owner.on_update)
+    rpc_register.remove_callback(owner.on_update)
+    assert len(rpc_register.callbacks) == callbacks_before
+
+    # Repeated removal and removing an unknown callback are no-ops.
+    rpc_register.remove_callback(owner.on_update)
+    rpc_register.remove_callback(_CallbackOwner().on_update)
+    assert len(rpc_register.callbacks) == callbacks_before
+
+    rpc_register.broadcast()
+    assert owner.received == []
+
+
+def test_bound_method_identity_across_method_objects(rpc_register):
+    """Two bound-method objects for the same method of the same instance must
+    be treated as the same callback (remove works with a fresh method object)."""
+    owner = _CallbackOwner()
+    rpc_register.add_callback(owner.on_update)
+    # 'owner.on_update' here creates a *new* bound-method object.
+    rpc_register.remove_callback(owner.on_update)
+
+    rpc_register.broadcast()
+    assert owner.received == []
+
+
+def test_callback_does_not_keep_owner_alive(rpc_register):
+    """Regression test for BW-009: the register must not keep callback owners
+    alive, and dead callbacks must be pruned on the next broadcast."""
+    import gc
+    import weakref
+
+    owner = _CallbackOwner()
+    rpc_register.add_callback(owner.on_update)
+    ref = weakref.ref(owner)
+    callbacks_with_owner = len(rpc_register.callbacks)
+
+    del owner
+    gc.collect()
+    assert ref() is None, "register must not hold a strong reference to the owner"
+
+    rpc_register.broadcast()  # must not raise; prunes the dead reference
+    assert len(rpc_register.callbacks) == callbacks_with_owner - 1