From adfb5e1151c36dce2ee8a82f311e0b29c3ad3e79 Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Tue, 25 Aug 2026 14:21:36 +0800 Subject: [PATCH] Unify asynchronous dialog ownership Signed-off-by: Loren Eteval --- .../qt-pyside6-object-lifetime-guidelines.md | 6 +-- Furious/Qt/AGENTS.md | 4 +- Furious/Qt/QtWidgets.py | 45 ++----------------- tests/test_dialog_geometry.py | 15 +++++-- tests/test_layout_matrix.py | 4 +- tests/test_qt_lifetime.py | 1 - tests/test_ui_behavior.py | 5 ++- tests/test_very_heavy.py | 6 +-- 8 files changed, 27 insertions(+), 59 deletions(-) diff --git a/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md b/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md index 782b3b8..ed95807 100644 --- a/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md +++ b/.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md @@ -360,9 +360,9 @@ Reusable dialogs follow a different policy. A reusable dialog is normally hidden `WA_DeleteOnClose` merely to make cleanup uniform. Furious implements these policies through `AppQDialog`, `AppQTransientDialog`, and -`AppQMessageBox`. `AppQMessageBox` currently has an additional registry because its -QMessageBox-compatible `open()` path bypasses the base implementation; its cleanup must -remain synchronized with the base dialog policy or be deliberately consolidated. +`AppQMessageBox`. Their asynchronous `open()` paths share the `AppQDialog` lifetime +registry; message-box presentation behavior must delegate ownership to that base path +rather than introduce a parallel registry. ## 14. Packaged and Compiled Builds Require Extra Caution diff --git a/Furious/Qt/AGENTS.md b/Furious/Qt/AGENTS.md index 810be29..b03394f 100644 --- a/Furious/Qt/AGENTS.md +++ b/Furious/Qt/AGENTS.md @@ -9,8 +9,8 @@ Use the `manage-qt-pyside6-lifetimes` skill for Qt ownership or lifecycle work. - Pass source text at construction when a control retains it for retranslation. Preserve focus, keyboard, shortcuts, accessibility, translated-text growth, high-DPI, responsive layout, and both themes. - `AppQMessageBox.windowTitle` is native metadata. Visible hierarchy is `heading`, `text`, then `informativeText`. - `AppQMessageBox.open()` bypasses `AppQDialog.open()`, so its separate message-box registry is its actual async owner; - keep that registry and the base destroyed cleanup balanced. + `AppQMessageBox.open()` delegates asynchronous ownership to `AppQDialog.open()`; do not add a parallel message-box + registry or release a transient box before native destruction completes. - `AppStyleSheet` is the public style authority. `StyleSheets` contains internal QSS fragments that consume the semantic palette; application code does not import fragments directly. diff --git a/Furious/Qt/QtWidgets.py b/Furious/Qt/QtWidgets.py index 32f1c76..7fe9d31 100644 --- a/Furious/Qt/QtWidgets.py +++ b/Furious/Qt/QtWidgets.py @@ -943,12 +943,6 @@ class AppQMessageBox(AppQTransientDialog): MultipleActionBaseWidth = 520 MaximumSurfaceWidth = 720 - # AppQDialog already wires a cleanup callback for its asynchronous registry. - # This message-box-specific registry repeats that cleanup so open() can bypass - # AppQDialog.open() while preserving QMessageBox-compatible show behavior. - # The duplicate cleanup is harmless and idempotent, but future changes must - # keep both registries synchronized or consolidate them deliberately. - _openMessageBoxes = {} _standardButtonOrder = ( StandardButton.Ok, StandardButton.Save, @@ -970,19 +964,6 @@ class AppQMessageBox(AppQTransientDialog): StandardButton.RestoreDefaults, ) - @staticmethod - def _releaseOpenMessageBox(key, *_args): - """Release an asynchronously opened message box after destruction.""" - AppQMessageBox._openMessageBoxes.pop(key, None) - - @staticmethod - def _scheduleOpenMessageBoxRelease(key, *_args): - """Release an async message box after destroyed-signal dispatch.""" - QtCore.QTimer.singleShot( - 0, - functools.partial(AppQMessageBox._releaseOpenMessageBox, key), - ) - def __init__(self, *args, **kwargs): """Initialize the message box while preserving QMessageBox arguments.""" windowTitle = kwargs.pop('windowTitle', None) @@ -1011,7 +992,6 @@ class AppQMessageBox(AppQTransientDialog): super().__init__(parent=parent, **kwargs) - self._lifetimeKey = object() self._windowMask = None self._icon = self.Icon.NoIcon self._heading = '' @@ -1026,15 +1006,6 @@ class AppQMessageBox(AppQTransientDialog): self._clickedButton = None self._handlingButton = False - scheduleRelease = functools.partial( - AppQMessageBox._scheduleOpenMessageBoxRelease, - self._lifetimeKey, - ) - - # Message boxes are delete-on-close. Their asynchronous owner releases - # them only after native destruction and its signal dispatch complete. - self.destroyed.connect(scheduleRelease) - self.setObjectName('AppMessageBox') windowFlags = ( @@ -1821,20 +1792,10 @@ class AppQMessageBox(AppQTransientDialog): return QDialog.exec(self) def open(self): - """Open and retain the message box until it finishes or is destroyed.""" - key = self._lifetimeKey - AppQMessageBox._openMessageBoxes[key] = self + """Prepare and open using the shared transient-dialog lifetime owner.""" + self._prepareForPresentation() - try: - self._prepareForPresentation() - - return QDialog.open(self) - except Exception: - # Any non-exit exceptions - - AppQMessageBox._releaseOpenMessageBox(key) - - raise + return super().open() def done(self, result): """Release the dimming mask before completing the transient dialog.""" diff --git a/tests/test_dialog_geometry.py b/tests/test_dialog_geometry.py index 2983ce8..30ec64a 100644 --- a/tests/test_dialog_geometry.py +++ b/tests/test_dialog_geometry.py @@ -123,11 +123,10 @@ class DialogGeometryTest(unittest.TestCase): application() def tearDown(self): - """Drain deferred deletion and require both async registries to settle.""" + """Drain deferred deletion and require async ownership to settle.""" collectAtBoundary() self.assertEqual(AppQDialog._openDialogs, {}) - self.assertEqual(AppQMessageBox._openMessageBoxes, {}) def dispose(self, dialog): """Destroy a reusable fixture after its assertion scope.""" @@ -309,17 +308,25 @@ class DialogGeometryTest(unittest.TestCase): buttons=AppQMessageBox.StandardButton.Ok, ) key = openBox._lifetimeKey + heldAtFinished = [] + openBox.finished.connect( + lambda _result: heldAtFinished.append(key in AppQDialog._openDialogs) + ) openBox.open() processQtEvents() self.assertEqual(openEvents, [True]) - self.assertIn(key, AppQMessageBox._openMessageBoxes) + self.assertIn(key, AppQDialog._openDialogs) openBox.accept() + + self.assertEqual(heldAtFinished, [True]) + self.assertIn(key, AppQDialog._openDialogs) + collectAtBoundary() - self.assertNotIn(key, AppQMessageBox._openMessageBoxes) + self.assertNotIn(key, AppQDialog._openDialogs) execEvents = [] execBox = CountingMessageBox( diff --git a/tests/test_layout_matrix.py b/tests/test_layout_matrix.py index 2fc7cc7..75fff4b 100644 --- a/tests/test_layout_matrix.py +++ b/tests/test_layout_matrix.py @@ -28,7 +28,7 @@ class IsolatedDisplayMatrixTest(unittest.TestCase): """Keep navigation and transient dialogs stable across supported scales.""" Script = r""" -from Furious.Qt import AppQMessageBox, AppStyleSheet +from Furious.Qt import AppQDialog, AppQMessageBox, AppStyleSheet from Furious.Widget import NavigationView from PySide6 import QtCore @@ -96,7 +96,7 @@ navigation.close() navigation.deleteLater() collectAtBoundary() -assert not AppQMessageBox._openMessageBoxes +assert not AppQDialog._openDialogs """ def testScaleAndThemeMatrix(self): diff --git a/tests/test_qt_lifetime.py b/tests/test_qt_lifetime.py index 9ea50ad..e1af9f7 100644 --- a/tests/test_qt_lifetime.py +++ b/tests/test_qt_lifetime.py @@ -130,7 +130,6 @@ class QtLifetimeTest(unittest.TestCase): collectAtBoundary() self.assertEqual(AppQDialog._openDialogs, {}) - self.assertEqual(AppQMessageBox._openMessageBoxes, {}) def assertAllDestroyed(self, references, destroyed, expected): """Assert weak wrappers and native destroyed signals agree.""" diff --git a/tests/test_ui_behavior.py b/tests/test_ui_behavior.py index 81ca6dd..c65ec27 100644 --- a/tests/test_ui_behavior.py +++ b/tests/test_ui_behavior.py @@ -71,6 +71,7 @@ from Furious.Plugins.API import RoutingOption from Furious.Qt import ( AppHue, AppQComboBox, + AppQDialog, AppQMessageBox, AppQSwitch, AppStyleSheet, @@ -1631,7 +1632,7 @@ class DialogBehaviorTest(unittest.TestCase): dialog.deleteRule() dialog.editRule() - self.assertEqual(AppQMessageBox._openMessageBoxes, {}) + self.assertEqual(AppQDialog._openDialogs, {}) self.assertEqual(dialog.routing['rules'], []) dialog.closeWindowButton.click() @@ -1911,7 +1912,7 @@ class DialogBehaviorTest(unittest.TestCase): finished, [int(AppQMessageBox.StandardButton.Cancel)], ) - self.assertEqual(AppQMessageBox._openMessageBoxes, {}) + self.assertEqual(AppQDialog._openDialogs, {}) class SharedConnectionPresentationTest(unittest.TestCase): diff --git a/tests/test_very_heavy.py b/tests/test_very_heavy.py index 3f626bc..ce629c1 100644 --- a/tests/test_very_heavy.py +++ b/tests/test_very_heavy.py @@ -23,7 +23,7 @@ from Furious.Backends.ExternalCore import ConfigExternalCore, ExternalCoreProces from Furious.Interface import ApplicationRunner from Furious.Models import LogCategory from Furious.Plugins import FuriousPlugin, PluginMetadata, PluginRegistry -from Furious.Qt import AppQMessageBox, AppQTransientDialog +from Furious.Qt import AppQDialog, AppQMessageBox, AppQTransientDialog from Furious.Utility import AppMainProcess from Furious.Service import APPLICATION_LOG_CATEGORY, LogManager, MetricsHistory from Furious.Widget import NavigationView @@ -329,7 +329,7 @@ class VeryHeavyContractTest(unittest.TestCase): self.assertEqual(resourceSnapshot()['threads'], baseline['threads']) def testFiveHundredMessageBoxesLeaveNoRegistryEntries(self): - """Exercise the specialized asynchronous registry at release scale.""" + """Exercise shared asynchronous dialog ownership at release scale.""" references = [] for index in range(500): @@ -350,7 +350,7 @@ class VeryHeavyContractTest(unittest.TestCase): collectAtBoundary() self.assertTrue(all(reference() is None for reference in references)) - self.assertFalse(AppQMessageBox._openMessageBoxes) + self.assertFalse(AppQDialog._openDialogs) if __name__ == '__main__':