Fix message-box button lifetimes

Signed-off-by: Loren Eteval <loren.eteval@proton.me>
This commit is contained in:
Loren Eteval
2026-09-19 12:07:51 +08:00
parent 3831e6e017
commit b93d7bded1
2 changed files with 153 additions and 22 deletions
+54 -19
View File
@@ -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)
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:
+98 -2
View File
@@ -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