fix(bec widgets): always call cleanup of child widgets on cleanup

bec_widgets/utils/bec_connector.py

@@ -161,8 +161,6 @@ class BECConnector:
 
         # 2) Enforce unique objectName among siblings with the same BECConnector parent
         self.setParent(parent)
-        if isinstance(self.parent(), QObject) and hasattr(self, "cleanup"):
-            self.parent().destroyed.connect(self._run_cleanup_on_deleted_parent)
 
         # Error popups
         self.error_utility = ErrorPopupUtility()
@@ -186,24 +184,6 @@ class BECConnector:
         except:
             logger.error(f"Error getting parent_id for {self.__class__.__name__}")
 
-    def _run_cleanup_on_deleted_parent(self) -> None:
-        """
-        Run cleanup on the deleted parent.
-        This method is called when the parent is deleted.
-        """
-        if not hasattr(self, "cleanup"):
-            return
-        try:
-            if not self._destroyed:
-                self.cleanup()
-                self._destroyed = True
-        except Exception:
-            content = traceback.format_exc()
-            logger.info(
-                "Failed to run cleanup on deleted parent. "
-                f"This is not necessarily an error as the parent may be deleted before the child and includes already a cleanup. The following exception was raised:\n{content}"
-            )
-
     def change_object_name(self, name: str) -> None:
         """
         Change the object name of the widget. Unregister old name and register the new one.

bec_widgets/utils/bec_widget.py

@@ -3,6 +3,7 @@ from __future__ import annotations
 from typing import TYPE_CHECKING
 
 import darkdetect
+import shiboken6
 from bec_lib.logger import bec_logger
 from qtpy.QtCore import QObject, Slot
 from qtpy.QtWidgets import QApplication
@@ -102,6 +103,13 @@ class BECWidget(BECConnector):
             # All widgets need to call super().cleanup() in their cleanup method
             logger.info(f"Registry cleanup for widget {self.__class__.__name__}")
             self.rpc_register.remove_rpc(self)
+        children = self.findChildren(BECWidget)
+        for child in children:
+            if not shiboken6.isValid(child):
+                # If the child is not valid, it means it has already been deleted
+                continue
+            child.close()
+            child.deleteLater()
 
     def closeEvent(self, event):
         """Wrap the close even to ensure the rpc_register is cleaned up."""

bec_widgets/utils/compact_popup.py

@@ -259,12 +259,3 @@ class CompactPopupWidget(QWidget):
     @expand_popup.setter
     def expand_popup(self, popup: bool):
         self._expand_popup = popup
-
-    def closeEvent(self, event):
-        # Called by Qt, on closing - since the children widgets can be
-        # BECWidgets, it is good to explicitely call 'close' on them,
-        # to ensure proper resources cleanup
-        for child in self.container.findChildren(QWidget, options=Qt.FindDirectChildrenOnly):
-            child.close()
-
-        super().closeEvent(event)

bec_widgets/widgets/containers/dock/dock.py

@@ -389,6 +389,7 @@ class BECDock(BECWidget, Dock):
         if widget in self.widgets:
             self.widgets.remove(widget)
         widget.close()
+        widget.deleteLater()
 
     def delete_all(self):
         """

tests/end-2-end/test_rpc_widgets_e2e.py

@@ -81,6 +81,7 @@ def test_available_widgets(qtbot, connected_client_gui_obj):
     # Number of top level widgets, should be 4
     top_level_widgets_count = 12
     assert len(gui._server_registry) == top_level_widgets_count
+    names = set(list(gui._server_registry.keys()))
     # Number of widgets with parent_id == None, should be 2
     widgets = [
         widget for widget in gui._server_registry.values() if widget["config"]["parent_id"] is None
@@ -151,7 +152,7 @@ def test_available_widgets(qtbot, connected_client_gui_obj):
             raise RuntimeError(
                 f"Widget {object_name} was not removed properly. The number of top level widgets "
                 f"is {len(gui._server_registry)} instead of {top_level_widgets_count}. The following "
-                f"widgets are still registered: {list(gui._server_registry.keys())}."
+                f"widgets are not cleaned up: {set(gui._server_registry.keys()) - names}"
             ) from exc
         # Number of widgets with parent_id == None, should be 2
         widgets = [

tests/unit_tests/test_compact_popup_widget.py

@@ -31,13 +31,6 @@ def compact_popup(qtbot):
     yield widget
 
 
-def test_widget_closing(qtbot, compact_popup):
-    with mock.patch.object(compact_popup.contained, "close") as close_method:
-        compact_popup.close()
-    qtbot.waitUntil(lambda: not compact_popup.isVisible(), timeout=1000)
-    close_method.assert_called_once()
-
-
 def test_size_policy(compact_popup):
     csp = compact_popup.sizePolicy()
     assert csp.horizontalPolicy() == QSizePolicy.Expanding