diff --git a/Furious/Qt/QtWidgets.py b/Furious/Qt/QtWidgets.py index 1ed9aa8..36934c3 100644 --- a/Furious/Qt/QtWidgets.py +++ b/Furious/Qt/QtWidgets.py @@ -32,6 +32,8 @@ from PySide6 import QtCore from PySide6.QtGui import * from PySide6.QtWidgets import * +from shiboken6 import isValid + from typing import Union import functools @@ -1126,6 +1128,7 @@ class AppQMessageBox(AppQTransientDialog): self._standardButtons = self.StandardButton.NoButton self._standardButtonMap = {} self._buttonRoles = {} + self._buttonConnections = {} self._defaultButton = None self._escapeButton = None self._clickedButton = None @@ -1264,15 +1267,7 @@ class AppQMessageBox(AppQTransientDialog): button.setMinimumHeight(34) button.setAttribute(QtCore.Qt.WidgetAttribute.WA_LayoutUsesWidgetRect) - connectWeakly( - button.clicked, - self, - '_handleButtonClicked', - sender=button, - forwardSender=True, - ) - - self._buttonRoles[button] = role + self._registerButton(button, role) if standardButton is not None: self._standardButtonMap[standardButton] = button @@ -1281,6 +1276,33 @@ class AppQMessageBox(AppQTransientDialog): return button + def _registerButton(self, button, role): + """Own exactly one click and destruction connection per attached button.""" + if button not in self._buttonConnections: + self._buttonConnections[button] = ( + connectWeakly( + button.clicked, + self, + '_handleButtonClicked', + sender=button, + forwardSender=True, + ), + connectWeakly( + button.destroyed, + self, + '_removeDestroyedButtons', + sender=button, + ), + ) + + self._buttonRoles[button] = role + + def _removeDestroyedButtons(self): + """Release registrations even while an invalid Python wrapper survives.""" + for button in tuple(self._buttonRoles): + if not isValid(button): + self.removeButton(button) + def _rebuildButtonLayout(self): """Lay out actions with Fluent-style margins and equal stretch.""" while self.buttonLayout.count(): @@ -1529,15 +1551,7 @@ class AppQMessageBox(AppQTransientDialog): customButton.setParent(self.buttonFrame) customButton.setAttribute(QtCore.Qt.WidgetAttribute.WA_LayoutUsesWidgetRect) - connectWeakly( - customButton.clicked, - self, - '_handleButtonClicked', - sender=customButton, - forwardSender=True, - ) - - self._buttonRoles[customButton] = role + self._registerButton(customButton, role) self._rebuildButtonLayout() return customButton @@ -1555,14 +1569,31 @@ class AppQMessageBox(AppQTransientDialog): def removeButton(self, button): """Remove one custom or standard button.""" - self._buttonRoles.pop(button, None) + if button not in self._buttonRoles: + return + + self._buttonRoles.pop(button) + + for connection in self._buttonConnections.pop(button): + QtCore.QObject.disconnect(connection) + + if self._defaultButton is button: + self._defaultButton = None + + if self._escapeButton is button: + self._escapeButton = None + + if self._clickedButton is button: + self._clickedButton = None for standardButton, candidate in tuple(self._standardButtonMap.items()): if candidate is button: self._standardButtons &= ~standardButton self._standardButtonMap.pop(standardButton, None) - button.setParent(None) + if isValid(button): + button.setParent(None) + self._rebuildButtonLayout() def buttons(self): @@ -1646,6 +1677,10 @@ class AppQMessageBox(AppQTransientDialog): finally: self._handlingButton = False + # A button listener may synchronously destroy this box or its parent. + if not isValid(self): + return + standardButton = self.standardButton(button) if standardButton != self.StandardButton.NoButton: diff --git a/tests/test_qt_lifetime.py b/tests/test_qt_lifetime.py index 350d11f..e5ce3b9 100644 --- a/tests/test_qt_lifetime.py +++ b/tests/test_qt_lifetime.py @@ -63,9 +63,9 @@ from Furious.Widget.ServerTableView import DeleteServersProgressDialog from PySide6 import QtCore from PySide6.QtGui import QImage -from PySide6.QtWidgets import QWidget +from PySide6.QtWidgets import QPushButton, QWidget -from shiboken6 import isValid +from shiboken6 import isValid, delete as deleteQObject from tests.support import ( application, @@ -149,6 +149,102 @@ class DelayedReceiver(QtCore.QObject): class QtLifetimeTest(unittest.TestCase): """Stress direct destruction evidence without relying on process RSS alone.""" + def testRemovedMessageBoxButtonDisconnectsAndCanBeReused(self): + """Detaching a button ends only the box-owned signal and role lifetime.""" + application() + with isolatedSettings(): + box = AppQMessageBox() + button = QPushButton('Reusable button') + finished = [] + externalClicks = [] + box.finished.connect(finished.append) + button.clicked.connect(lambda: externalClicks.append(True)) + try: + for _ in range(30): + box.addButton(button, box.ButtonRole.AcceptRole) + box.setDefaultButton(button) + box.setEscapeButton(button) + box.removeButton(button) + button.click() + self.assertEqual(finished, []) + self.assertIsNone(box.defaultButton()) + self.assertIsNone(box.escapeButton()) + self.assertIsNone(button.parent()) + self.assertEqual(button.receivers(QtCore.SIGNAL('clicked()')), 1) + + self.assertEqual(len(externalClicks), 30) + box.addButton(button, box.ButtonRole.AcceptRole) + box.addButton(button, box.ButtonRole.AcceptRole) + box.open() + button.click() + processQtEvents() + self.assertEqual(finished, [int(AppQDialog.DialogCode.Accepted)]) + self.assertFalse(isValid(box)) + self.assertFalse(isValid(button)) + finally: + if isValid(box): + deleteQObject(box) + if isValid(button): + deleteQObject(button) + processQtEvents() + + def testReplacingMessageBoxButtonsReleasesOldDefaultAndEscape(self): + """A retained dialog must not retain removed standard-button wrappers.""" + application() + with isolatedSettings(): + box = AppQMessageBox() + references = [] + try: + for _ in range(30): + box.setStandardButtons(box.StandardButton.Yes) + button = box.button(box.StandardButton.Yes) + references.append(weakref.ref(button)) + box.setDefaultButton(button) + box.setEscapeButton(button) + box.setStandardButtons(box.StandardButton.No) + del button + processQtEvents() + self.assertIsNone(box.defaultButton()) + self.assertIsNone(box.escapeButton()) + self.assertTrue(all(reference() is None for reference in references)) + finally: + deleteQObject(box) + processQtEvents() + + def testNativeButtonDestructionRemovesMessageBoxRegistrations(self): + """Deleting an attached button cannot leave invalid wrapper roles behind.""" + application() + with isolatedSettings(): + box = AppQMessageBox() + try: + for _ in range(30): + button = box.addButton(box.StandardButton.Yes) + box.setDefaultButton(button) + box.setEscapeButton(button) + deleteQObject(button) + self.assertEqual(box.buttons(), []) + self.assertIsNone(box.defaultButton()) + self.assertIsNone(box.escapeButton()) + self.assertIsNone(box.button(box.StandardButton.Yes)) + finally: + deleteQObject(box) + processQtEvents() + + def testMessageBoxCallbackMayDestroyItsOwner(self): + """Button activation cannot finish a box destroyed by its own listener.""" + application() + with isolatedSettings(): + owner = QWidget() + box = AppQMessageBox(parent=owner) + button = box.addButton(box.StandardButton.Yes) + box.buttonClicked.connect(lambda *_args: deleteQObject(owner)) + with mock.patch('sys.excepthook') as exceptionHook: + button.click() + processQtEvents() + exceptionHook.assert_not_called() + self.assertFalse(isValid(box)) + self.assertFalse(isValid(button)) + def testRoutingDocumentationDoesNotEnterCompiledMethodProtection(self): """Closing routing editors releases labels under Nuitka-style retention.""" originalConnect = QtCore.SignalInstance.connect