From abb819a27e2cfa3e45041f2186702784be4350c7 Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Sat, 19 Sep 2026 11:12:15 +0800 Subject: [PATCH] Tie confirmations to their owning views Signed-off-by: Loren Eteval --- Furious/Backends/Xray/AssetListView.py | 15 ++-- Furious/Backends/Xray/RoutingWindow.py | 23 +++--- Furious/Widget/AGENTS.md | 3 + Furious/Widget/ServerTableView.py | 13 ++-- Furious/Widget/SubscriptionTableView.py | 13 ++-- tests/test_qt_lifetime.py | 95 +++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 39 deletions(-) diff --git a/Furious/Backends/Xray/AssetListView.py b/Furious/Backends/Xray/AssetListView.py index 801de93..80a97f0 100644 --- a/Furious/Backends/Xray/AssetListView.py +++ b/Furious/Backends/Xray/AssetListView.py @@ -204,7 +204,7 @@ class XrayAssetListView(Mixins.ThemeAware, AppQListView): # Do not overwrite pass - mbox = MBoxAssetExists(icon=AppQMessageBox.Icon.Question) + mbox = MBoxAssetExists(icon=AppQMessageBox.Icon.Question, parent=self) mbox.setText(_('Asset file already exists. Overwrite?')) mbox.setInformativeText(basename) mbox.finished.connect(functools.partial(handleResultCode, filename)) @@ -241,14 +241,11 @@ class XrayAssetListView(Mixins.ThemeAware, AppQListView): # Do not delete pass - if PLATFORM == 'Windows': - # Windows - mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question) - else: - # macOS & linux - mbox = MBoxQuestionDelete( - icon=AppQMessageBox.Icon.Question, parent=self.parent() - ) + # The completion callback uses this view. Native view destruction must + # also end the pending confirmation, even while its window survives. + mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question, parent=self) + + if PLATFORM != 'Windows': mbox.setWindowModality(QtCore.Qt.WindowModality.WindowModal) mbox.isMulti = bool(len(filenames) > 1) diff --git a/Furious/Backends/Xray/RoutingWindow.py b/Furious/Backends/Xray/RoutingWindow.py index e0ba99f..490de86 100644 --- a/Furious/Backends/Xray/RoutingWindow.py +++ b/Furious/Backends/Xray/RoutingWindow.py @@ -1109,11 +1109,10 @@ class RoutingRulesDialog(AppQTransientDialog): # This prompt is subordinate to a transient editor on every platform. # Native owner destruction must also end its pending confirmation. - mbox = MBoxQuestionDelete( - icon=AppQMessageBox.Icon.Question, - parent=self, - ) - mbox.setWindowModality(QtCore.Qt.WindowModality.WindowModal) + mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question, parent=self) + + if PLATFORM != 'Windows': + mbox.setWindowModality(QtCore.Qt.WindowModality.WindowModal) mbox.isMulti = bool(len(indexes) > 1) mbox.possibleRemark = self.listView.selectedRuleText() @@ -1305,15 +1304,11 @@ class UserRoutingTableView(Mixins.QTranslatable, AppQTableView): self.flushAll() - if PLATFORM == 'Windows': - # Windows - mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question) - else: - # macOS & linux - mbox = MBoxQuestionDelete( - icon=AppQMessageBox.Icon.Question, - parent=self, - ) + # The completion callback uses this view. Native view destruction must + # also end the pending confirmation, even while its window survives. + mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question, parent=self) + + if PLATFORM != 'Windows': mbox.setWindowModality(QtCore.Qt.WindowModality.WindowModal) mbox.isMulti = bool(len(indexes) > 1) diff --git a/Furious/Widget/AGENTS.md b/Furious/Widget/AGENTS.md index 0556162..ba27620 100644 --- a/Furious/Widget/AGENTS.md +++ b/Furious/Widget/AGENTS.md @@ -30,6 +30,9 @@ some service owners; that construction detail does not make every view an indepe - Models, delegates, headers, menus, actions, animations, spinners, WebEngine/map objects, timers, workers, and replies each need one owner. Persistent widgets connect once and refresh state; visibility may pause rendering/animation, not application-level log draining, traffic collection, or other service ownership. +- A pending confirmation whose callback mutates a view belongs to that exact view on every platform. A shared + window parent can outlive the view, and an unparented prompt can outlive both. Native view destruction must end + the prompt without running its mutation; `test_qt_lifetime.py` exercises this with the containing window still alive. - Model notifications describe the real source mutation. Structural replacement may legitimately use a model reset; metadata-only test results should update the exact cell. Do not use resets/full repaints to mask broken mapping or missing identity restoration. Test selected identities and the current keyboard index independently. diff --git a/Furious/Widget/ServerTableView.py b/Furious/Widget/ServerTableView.py index b8f27a0..4d9229e 100644 --- a/Furious/Widget/ServerTableView.py +++ b/Furious/Widget/ServerTableView.py @@ -1784,14 +1784,11 @@ class ServerTableView( else: pass - if PLATFORM == 'Windows': - # Windows - mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question) - else: - # macOS & linux - mbox = MBoxQuestionDelete( - icon=AppQMessageBox.Icon.Question, parent=self.parent() - ) + # The completion callback uses this view. Native view destruction must + # also end the pending confirmation, even while its window survives. + mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question, parent=self) + + if PLATFORM != 'Windows': mbox.setWindowModality(QtCore.Qt.WindowModality.WindowModal) mbox.isMulti = bool(len(indexes) > 1) diff --git a/Furious/Widget/SubscriptionTableView.py b/Furious/Widget/SubscriptionTableView.py index 6030099..9a734dc 100644 --- a/Furious/Widget/SubscriptionTableView.py +++ b/Furious/Widget/SubscriptionTableView.py @@ -770,14 +770,11 @@ class SubscriptionTableView(Mixins.QTranslatable, AppQTableView): # Do not delete pass - if PLATFORM == 'Windows': - # Windows - mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question) - else: - # macOS & linux - mbox = MBoxQuestionDelete( - icon=AppQMessageBox.Icon.Question, parent=self.parent() - ) + # The completion callback uses this view. Native view destruction must + # also end the pending confirmation, even while its window survives. + mbox = MBoxQuestionDelete(icon=AppQMessageBox.Icon.Question, parent=self) + + if PLATFORM != 'Windows': mbox.setWindowModality(QtCore.Qt.WindowModality.WindowModal) mbox.isMulti = bool(len(indexes) > 1) diff --git a/tests/test_qt_lifetime.py b/tests/test_qt_lifetime.py index 24017e2..169bcbd 100644 --- a/tests/test_qt_lifetime.py +++ b/tests/test_qt_lifetime.py @@ -23,7 +23,9 @@ from Furious.Backends.ExternalCore.Editor import ExternalCoreEditor from Furious.Backends.Hysteria1.Editor import Hysteria1Editor from Furious.Backends.Hysteria2.Editor import Hysteria2Editor from Furious.Backends.Hysteria2.TunSettingsDialog import Hysteria2TunSettingsDialog +from Furious.Backends.Xray.AssetListView import XrayAssetListView from Furious.Backends.Xray.RoutingWindow import ( + UserRoutingTableView, RoutingDocumentationURL, RoutingPreviewDialog, RoutingRuleEditDialog, @@ -37,6 +39,10 @@ from Furious.Backends.Xray.VlessEditor import VlessEditor from Furious.Backends.Xray.VmessEditor import VmessEditor from Furious.Actions.Import import ImportURIsProgressDialog from Furious.Frozenlib import Mixins +from Furious.Models import CoreConfiguration, ServerProfile +from Furious.Repository import Storage +from Furious.Widget.ServerTableView import ServerTableView +from Furious.Widget.SubscriptionTableView import SubscriptionTableView from Furious.Qt import ( AppQAction, AppQDialog, @@ -72,6 +78,8 @@ from tests.support import ( ) import gc +from pathlib import Path +import tempfile import unittest from unittest import mock import weakref @@ -249,6 +257,93 @@ class QtLifetimeTest(unittest.TestCase): self.assertAllDestroyed(references, destroyed, 60) + def testViewConfirmationsDieWithTheirCallbackOwner(self): + """Deleting a view also destroys every prompt that could mutate it.""" + profile = ServerProfile.fromConfiguration( + CoreConfiguration({'type': 'fixture'}), {'displayName': 'Keep'} + ) + profiles = [profile] + subscriptions = {'fixture': {'remark': 'Keep', 'enabled': False}} + routings = {'fixture': {'remark': 'Keep', 'rules': []}} + + with ( + isolatedSettings(), + mock.patch.object(Storage, 'UserServers', lambda: profiles), + mock.patch.object(Storage, 'UserSubs', lambda: subscriptions), + mock.patch.object(Storage, 'UserRoutings', lambda: routings), + tempfile.TemporaryDirectory() as directory, + mock.patch( + 'Furious.Backends.Xray.AssetListView.XRAY_ASSET_DIR', Path(directory) + ), + ): + asset = Path(directory) / 'fixture.dat' + asset.write_bytes(b'keep') + + for platform in ('Windows', 'Linux', 'Darwin'): + for family in ( + 'servers', + 'subscriptions', + 'routings', + 'assetDelete', + 'assetOverwrite', + ): + with self.subTest(platform=platform, family=family): + references = [] + destroyed = [] + for _ in range(20): + parent = QWidget() + if family == 'servers': + view = ServerTableView( + parent=parent, + configurationEditorFactory=QWidget, + qrCodeWindowFactory=QWidget, + importActionsFactory=tuple, + ) + elif family == 'subscriptions': + view = SubscriptionTableView(parent=parent) + elif family == 'routings': + view = UserRoutingTableView(parent=parent) + else: + view = XrayAssetListView(parent=parent) + + view.setCurrentIndex(view.model().index(0, 0)) + module = type(view).__module__ + with mock.patch(module + '.PLATFORM', platform): + if family == 'assetOverwrite': + view.appendNewItem(str(asset)) + else: + view.deleteSelectedItem() + + confirmation = next(iter(AppQDialog._openDialogs.values())) + references.append(weakref.ref(confirmation)) + confirmation.destroyed.connect( + lambda *_args: destroyed.append(True) + ) + + if isinstance(view, ServerTableView): + view.cleanup() + view.configurationEditor.deleteLater() + + try: + view.deleteLater() + processQtEvents() + self.assertFalse(isValid(view)) + self.assertFalse(isValid(confirmation)) + self.assertTrue(isValid(parent)) + self.assertEqual(profiles, [profile]) + self.assertIn('fixture', subscriptions) + self.assertIn('fixture', routings) + self.assertEqual(asset.read_bytes(), b'keep') + finally: + if isValid(confirmation): + confirmation.reject() + parent.deleteLater() + processQtEvents() + + del confirmation, view, parent + + self.assertAllDestroyed(references, destroyed, 20) + def testReopenedDialogSurvivesPreviousPresentationCleanup(self): """A queued finish must not release the next asynchronous presentation.""" destroyed = []