Better disconnect cleanup on qmenus

pyproject.toml

@@ -27,7 +27,7 @@ classifiers = [
 ]
 dynamic = ["version"]
 dependencies = [
-    "psygnal>=0.3.4",
+    "psygnal>=0.9.5",
     "pydantic>=1.8",
     "pydantic-compat>=0.1.1",
     "in-n-out>=0.1.5",

src/app_model/backends/qt/_qmenu.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import contextlib
 from typing import TYPE_CHECKING, Collection, Iterable, Mapping, Sequence, cast
 
 from qtpy.QtWidgets import QMenu, QMenuBar, QToolBar
@@ -52,7 +51,11 @@ def __init__(
         self.setObjectName(menu_id)
         self.rebuild()
         self._app.menus.menus_changed.connect(self._on_registry_changed)
-        self.destroyed.connect(self._disconnect)
+
+        @self.destroyed.connect  # type: ignore
+        def _disconnect() -> None:
+            self._app.menus.menus_changed.disconnect(self._on_registry_changed)
+
         # ----------------------
 
         if title is not None:
@@ -106,15 +109,9 @@ def _on_about_to_show(self) -> None:
             if isinstance(action, QCommandRuleAction):
                 action._refresh()
 
-    def _disconnect(self) -> None:
-        self._app.menus.menus_changed.disconnect(self._on_registry_changed)
-
     def _on_registry_changed(self, changed_ids: set[str]) -> None:
         if self._menu_id in changed_ids:
-            # if this (sub)menu has been removed from the registry,
-            # we may hit a RuntimeError when trying to rebuild it.
-            with contextlib.suppress(RuntimeError):
-                self.rebuild()
+            self.rebuild()
 
 
 class QModelSubmenu(QModelMenu):
@@ -192,7 +189,11 @@ def __init__(
         self.setObjectName(menu_id)
         self.rebuild()
         self._app.menus.menus_changed.connect(self._on_registry_changed)
-        self.destroyed.connect(self._disconnect)
+
+        @self.destroyed.connect  # type: ignore
+        def _disconnect() -> None:
+            self._app.menus.menus_changed.disconnect(self._on_registry_changed)
+
         # ----------------------
 
         if title is not None:
@@ -243,9 +244,6 @@ def rebuild(
             exclude=self._exclude if exclude is None else exclude,
         )
 
-    def _disconnect(self) -> None:
-        self._app.menus.menus_changed.disconnect(self._on_registry_changed)
-
     def _on_registry_changed(self, changed_ids: set[str]) -> None:
         if self._menu_id in changed_ids:
             self.rebuild()

src/app_model/registries/_menus_reg.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+from logging import getLogger
 from typing import TYPE_CHECKING, Any, Callable, Final, Iterable, Iterator, Sequence
 
 from psygnal import Signal
@@ -10,6 +11,7 @@
     from app_model.types import DisposeCallable, MenuOrSubmenu
 
 MenuId = str
+logger = getLogger(__name__)
 
 
 class MenusRegistry:
@@ -55,7 +57,10 @@
             for id_ in changed_ids:
                 if not self._menu_items.get(id_):
                     del self._menu_items[id_]
-            self.menus_changed.emit(changed_ids)
+            try:
+                self.menus_changed.emit(changed_ids)
+            except Exception as e:
+                logger.exception("Error during menu disposal: %s", e)
 
         if changed_ids:
             self.menus_changed.emit(changed_ids)

tests/test_qt/test_qmenu.py

@@ -64,8 +64,6 @@ def test_menu(
     with pytest.raises(NameError, match="Names required to eval this expression"):
         menu.update_from_context({})
 
-    menu._disconnect()
-
 
 def test_submenu(qtbot: QtBot, full_app: FullApp) -> None:
     app = full_app