From 1881cb6aab76128d71b7bd6461eebffe75db77cd Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Wed, 7 Oct 2026 20:42:21 +0800 Subject: [PATCH] Fix Qt object lifetime handling Signed-off-by: Loren Eteval --- Furious/Actions/Import.py | 3 + Furious/Backends/ExternalCore/Editor.py | 3 + Furious/Backends/Xray/AssetWindow.py | 3 + Furious/Frozenlib/Mixins.py | 20 +- Furious/Qt/AGENTS.md | 13 +- Furious/Qt/QtWidgets.py | 8 +- Furious/Window/TextEditorWindow.py | 30 ++- tests/README.md | 5 +- tests/fixtures/editor_lifetime_probe.py | 177 +++++++++++++++ tests/test_frozenlib.py | 24 ++ tests/test_qt_lifetime.py | 282 +++++++++++++++++++++++- 11 files changed, 551 insertions(+), 17 deletions(-) diff --git a/Furious/Actions/Import.py b/Furious/Actions/Import.py index 4589f749..542608dc 100644 --- a/Furious/Actions/Import.py +++ b/Furious/Actions/Import.py @@ -399,6 +399,9 @@ class ImportFromFileAction(AppQAction): filter=_('Text files (*.json);;All files (*)'), ) + if not Mixins.qObjectIsValid(self): + return + if filename: try: with open(filename, 'r', encoding='utf-8') as file: diff --git a/Furious/Backends/ExternalCore/Editor.py b/Furious/Backends/ExternalCore/Editor.py index 8e6f1e0e..3d0e12bd 100644 --- a/Furious/Backends/ExternalCore/Editor.py +++ b/Furious/Backends/ExternalCore/Editor.py @@ -132,6 +132,9 @@ class ExternalCorePathInput(EditorWidgetBinding): _('All files (*)'), ) + if not Mixins.qObjectIsValid(self._container, self._input): + return + if selected: self._input.setText(str(Path(selected).resolve(strict=False))) diff --git a/Furious/Backends/Xray/AssetWindow.py b/Furious/Backends/Xray/AssetWindow.py index 66ad8169..b1c95e05 100644 --- a/Furious/Backends/Xray/AssetWindow.py +++ b/Furious/Backends/Xray/AssetWindow.py @@ -136,5 +136,8 @@ class XrayAssetWindow(AppQMainWindow): None, _('Import File'), filter=_('All files (*)') ) + if not Mixins.qObjectIsValid(self, self.xrayAssetListView): + return + if filename: self.xrayAssetListView.appendNewItem(filename) diff --git a/Furious/Frozenlib/Mixins.py b/Furious/Frozenlib/Mixins.py index 1c71778d..57b3cd63 100644 --- a/Furious/Frozenlib/Mixins.py +++ b/Furious/Frozenlib/Mixins.py @@ -111,15 +111,19 @@ class Mixins: """Group reusable lifecycle, translation, theme, and Qt context mixins.""" @staticmethod - def qObjectIsValid(qobject) -> bool: - """Return the q object is valid value used by the mixins.""" - if not isinstance(qobject, QtCore.QObject): - return True + def qObjectIsValid(qobject, *qobjects) -> bool: + """Require every QObject to be valid; accept non-QObjects as before.""" + for ob in (qobject, *qobjects): + if not isinstance(ob, QtCore.QObject): + continue - try: - return isValidQObject(qobject) - except RuntimeError: - return False + try: + if not isValidQObject(ob): + return False + except RuntimeError: + return False + + return True class ConnectionAware: """Represent connection aware.""" diff --git a/Furious/Qt/AGENTS.md b/Furious/Qt/AGENTS.md index 600e63a7..f9a29832 100644 --- a/Furious/Qt/AGENTS.md +++ b/Furious/Qt/AGENTS.md @@ -86,10 +86,10 @@ Use the `manage-qt-pyside6-lifetimes` skill for source lifetime work when availa does not enforce that requirement. An action also owns a submenu supplied without a QWidget parent and schedules its native deletion when the action dies; `QAction.setMenu()` alone does not establish parent ownership. Explicitly parented menus retain their chosen owner. -- `AppQMenu` adopts constructor actions/separator placeholders that lack a QObject parent; explicitly owned actions - remain borrowed. Adding an action to a menu or action group does not itself transfer QObject ownership. Dynamic - groups must parent their generated actions so retiring the group releases native actions even when compiled - callbacks keep Python wrappers alive. `tests/test_qt_lifetime.py` exercises both borrowed and owned cases. +- `AppQMenu` and `AppQToolBar` adopt constructor actions/separator placeholders without a QObject parent; explicitly + owned actions remain borrowed. Adding an action to a menu or action group does not itself transfer QObject + ownership. Dynamic groups must parent their generated actions so retiring the group releases native actions even + when compiled callbacks keep Python wrappers alive. `tests/test_qt_lifetime.py` exercises both borrowed and owned cases. - Every `QNetworkReply` has one manager/context owner, one freshness rule, and one terminal deletion path. Request context must also be released when native destruction skips `finished`, including manager-first teardown with retained Python wrappers. Use the shared network-manager tracking boundary; cleanup must not capture a reply @@ -99,6 +99,11 @@ Use the `manage-qt-pyside6-lifetimes` skill for source lifetime work when availa Once-only HTTP completion also guards an in-progress callback: a second reply may finish synchronously inside the first callback before the terminal flag is set. Preserve per-request completion mode and the existing final flag timing; `tests/test_service_runtime.py` exercises real nested finished delivery and native resource teardown. +- Modal `exec()` and native file/directory choosers run nested event loops. Recheck the initiating feature and + required native widgets after return before reading controls, opening files, mutating a model, or presenting + follow-up UI. A pure Python editor binding can survive its destroyed Qt field tree. Publication after a data + commit may also destroy the presenter: retain the committed outcome while stopping stale UI work, including + the close-confirmation caller. `ModalPickerLifetimeTest` in `tests/test_qt_lifetime.py` challenges these boundaries. - Queued delivery never transfers ownership implicitly. The sender may finish before delivery, so callbacks resolve a still-valid receiver and current generation in the receiver's Qt thread before touching widgets, models, or wrappers. A zero-delay timer yields work but does not establish ordering against an unrelated Qt event. Express required diff --git a/Furious/Qt/QtWidgets.py b/Furious/Qt/QtWidgets.py index 354e47a5..c32e57f2 100644 --- a/Furious/Qt/QtWidgets.py +++ b/Furious/Qt/QtWidgets.py @@ -2423,16 +2423,22 @@ class AppQToolBar(Mixins.QTranslatable, QToolBar): for action in actions: if isinstance(action, AppQSeparator): + if action.parent() is None: + action.setParent(self) + self._actions.append(action) self.addSeparator() elif isinstance(action, AppQAction): + if action.parent() is None: + action.setParent(self) + self._actions.append(action) self.addAction(action) else: # Do nothing pass - self.actionTriggered.connect(self.showMenuBelow) + connectWeakly(self.actionTriggered, self, 'showMenuBelow', sender=self) @QtCore.Slot(AppQAction) def showMenuBelow(self, action: AppQAction): diff --git a/Furious/Window/TextEditorWindow.py b/Furious/Window/TextEditorWindow.py index 23a521dc..b1ef8ba1 100644 --- a/Furious/Window/TextEditorWindow.py +++ b/Furious/Window/TextEditorWindow.py @@ -333,9 +333,15 @@ class TextEditorWindow(AppQMainWindow): pass + if not Mixins.qObjectIsValid(self): + return True + if index == Storage.UserActivatedItemIndex(): showMBoxNewChangesNextTime(parent=self, method=showChangesMethod) + if not Mixins.qObjectIsValid(self): + return True + self.markAsSaved() return True @@ -346,10 +352,15 @@ class TextEditorWindow(AppQMainWindow): None, _('Save File'), filter=_('Text files (*.json);;All files (*)') ) + if not Mixins.qObjectIsValid(self, self.jsonEditor): + return + if filename: try: + content = self.jsonEditor.toPlainText() + with open(filename, 'w', encoding='utf-8') as file: - file.write(self.jsonEditor.toPlainText()) + file.write(content) except Exception as ex: # Any non-exit exceptions @@ -437,7 +448,14 @@ class TextEditorWindow(AppQMainWindow): """Handle button clicked.""" if button == mbox.button0: # Save - if self.save(showChangesMethod='exec'): + saved = self.save(showChangesMethod='exec') + + if not Mixins.qObjectIsValid(self, mbox): + event.ignore() + + return + + if saved: mbox.close() event.accept() @@ -447,6 +465,11 @@ class TextEditorWindow(AppQMainWindow): # Discard self.markAsSaved() + if not Mixins.qObjectIsValid(self, mbox): + event.ignore() + + return + mbox.close() event.accept() @@ -459,6 +482,9 @@ class TextEditorWindow(AppQMainWindow): # Show the MessageBox and wait for the user to close it mbox.exec() + if not Mixins.qObjectIsValid(self): + return + if event.isAccepted(): super().closeEvent(event) else: diff --git a/tests/README.md b/tests/README.md index 2712a46c..5ef5eebe 100644 --- a/tests/README.md +++ b/tests/README.md @@ -134,7 +134,7 @@ worker. Choose tests by the changed contract rather than by filename alone. | [test_qr_export_scalability.py](test_qr_export_scalability.py) | Production capture cap, immediate single export, incremental yielding, failure/cancel/close paths, immutable snapshots, window-owned state destruction. | | [test_stylesheet_states.py](test_stylesheet_states.py) | Targeted rendering/alpha/geometry assertions for table/list insets, popup corners, clear buttons, focus/disabled states, and stylesheet composition. | | [test_theme_transition.py](test_theme_transition.py) | Real cross-fades, immediate theme activation, interruption, per-window resize/destruction, coordinator teardown, animation policy, native resize/deletion probes, deferred completion delivery, replacement/stop flushing and owner-first cancellation. | -| [test_qt_lifetime.py](test_qt_lifetime.py) | Native destruction and weak-wrapper/registry evidence across dialogs, menus, actions, timers, signals, message-box buttons/masks, owner-first confirmations, reusable editors, simulated compiled-method retention. | +| [test_qt_lifetime.py](test_qt_lifetime.py) | Modal chooser/notification continuations after owner teardown, text-export file preservation, toolbar owned/borrowed action teardown, native destruction and weak-wrapper/registry evidence across dialogs, menus, actions, timers, signals, message-box buttons/masks, owner-first confirmations, reusable editors, simulated compiled-method retention. | ### Repeated stress and release confidence @@ -358,6 +358,9 @@ destruction, statistics executor/thread release with an invalid wrapper retained release before native child-worker deletion. Controller-owned update requests die with their controller; injected services retain their existing owner. Source regressions also exercise reentrant metrics enablement, download cancellation with a deleted reply, and disposal during an output-drain callback. +The modal-picker probe repeats native owner destruction during file/directory selection and verifies no stale +UI operation or file truncation. The toolbar probe checks owned versus borrowed native actions and compiled +callback growth; wrapper collection is checked at the batch boundary, not forced per cycle. The publication/teardown probe covers nested once-only and per-request HTTP completion, native destruction during Home status publication, update notifications ending manager/parent ownership, and destruction or cancellation during the shared post-show event flush. Window/QR probes verify registry and timer cleanup before further work. diff --git a/tests/fixtures/editor_lifetime_probe.py b/tests/fixtures/editor_lifetime_probe.py index ccb1faf2..e1cf2099 100644 --- a/tests/fixtures/editor_lifetime_probe.py +++ b/tests/fixtures/editor_lifetime_probe.py @@ -20,6 +20,8 @@ from __future__ import annotations from Furious.Backends import OFFICIAL_PLUGIN_TYPES +from Furious.Backends.ExternalCore.Editor import ExternalCorePathInput +from Furious.Backends.Xray.AssetWindow import XrayAssetWindow from Furious.Backends.Xray.RoutingWindow import RoutingRulesDialog from Furious.Backends.Xray.AssetListView import XrayAssetListView import Furious.Backends.Xray.AssetListView as assetModule @@ -39,6 +41,8 @@ from Furious.Qt import ( AppQDialog, AppQMessageBox, AppQMainWindow, + AppQToolBar, + AppQSeparator, ThemeTransition, connectWeakly, ) @@ -72,6 +76,7 @@ from Furious.Repository import Storage from Furious.Window.SettingsPage import _TUNBackendSettingsCard from Furious.Window.HomePage import HomePage, NetworkStateBadge, AppConnectivityManager from Furious.Window.QRCodeWindow import QRCodeWindow +from Furious.Window.TextEditorWindow import TextEditorWindow from Furious.Widget.ServerTableView import ServerTableView import PySide6 @@ -95,6 +100,7 @@ from types import SimpleNamespace from unittest import mock import argparse +import importlib import json import sys import weakref @@ -1837,6 +1843,175 @@ def runConfirmationProbe(iterations=100): return result +def runToolbarOwnershipProbe(iterations=100): + """Toolbar teardown releases owned actions while preserving borrowed owners.""" + application() + protected = getattr( + sys.modules.get('PySide6-postLoad', PySide6), '_protected', None + ) + protectedBefore = len(protected) if protected is not None else None + references = [] + destroyed = [] + + with isolatedSettings(): + for borrowed in (False, True): + for _ in range(iterations): + owner = QWidget() + action = AppQAction( + 'Toolbar fixture', parent=owner if borrowed else None + ) + separator = AppQSeparator() + + if borrowed: + separator.setParent(owner) + + toolbar = AppQToolBar(action, separator, parent=owner) + references.append(weakref.ref(toolbar)) + toolbar.destroyed.connect(lambda *_args: destroyed.append(True)) + toolbar.actionTriggered.emit(action) + + deleteQObject(toolbar) + + assert isValid(action) == borrowed + assert isValid(separator) == borrowed + + deleteQObject(owner) + + assert not isValid(action) + assert not isValid(separator) + + del action, separator, toolbar, owner + + collectAtBoundary() + + assert len(destroyed) == iterations * 2 + assert all(reference() is None for reference in references) + + growth = len(protected) - protectedBefore if protected is not None else None + + if growth is not None: + assert growth == 0, growth + + return { + 'toolbarCycles': iterations * 2, + 'destroyed': len(destroyed), + 'protectedGrowth': growth, + } + + +def runModalPickerProbe(iterations=100): + """Exercise modal owner-first teardown with actual native widget deletion.""" + application() + references = [] + destroyed = [] + textModule = importlib.import_module('Furious.Window.TextEditorWindow') + assetWindowModule = importlib.import_module('Furious.Backends.Xray.AssetWindow') + pathModule = importlib.import_module('Furious.Backends.ExternalCore.Editor') + + def destroyOwner(owner): + loop = QtCore.QEventLoop() + + def finish(): + deleteQObject(owner) + loop.quit() + + QtCore.QTimer.singleShot(0, finish) + loop.exec() + + def record(object_): + references.append(weakref.ref(object_)) + object_.destroyed.connect(lambda *_args: destroyed.append(True)) + + with isolatedSettings(), tempfile.TemporaryDirectory() as directory: + filename = Path(directory) / 'fixture.json' + for _ in range(iterations): + filename.write_text('keep', encoding='utf-8') + parent = QWidget() + editor = TextEditorWindow(parent) + editor.jsonEditor.setPlainText('{"fixture": true}') + record(editor) + + def saveSelection(*_args, **_kwargs): + destroyOwner(parent) + return str(filename), '' + + with mock.patch.object( + textModule.QFileDialog, 'getSaveFileName', side_effect=saveSelection + ): + editor.saveAsFile() + + assert not isValid(editor) + assert filename.read_text(encoding='utf-8') == 'keep' + del editor, parent + + for directoryMode in (False, True): + binding = ExternalCorePathInput( + 'Path', 'executable', directory=directoryMode + ) + container = binding._container + record(container) + + def pathSelection(*_args, **_kwargs): + destroyOwner(container) + return str(filename) if directoryMode else (str(filename), '') + + method = 'getExistingDirectory' if directoryMode else 'getOpenFileName' + with mock.patch.object( + pathModule.QFileDialog, method, side_effect=pathSelection + ): + binding.browse() + + assert not isValid(binding._input) + deleteQObject(binding._title) + del binding, container + + parent = QWidget() + window = XrayAssetWindow(parent) + record(window) + + def assetSelection(*_args, **_kwargs): + destroyOwner(parent) + return str(filename), '' + + with mock.patch.object( + assetWindowModule.QFileDialog, + 'getOpenFileName', + side_effect=assetSelection, + ): + with mock.patch.object( + window.xrayAssetListView, 'appendNewItem' + ) as append: + window.appendNewItem() + append.assert_not_called() + assert not isValid(window) + del window, parent + + action = importModule.ImportFromFileAction() + record(action) + + def importSelection(*_args, **_kwargs): + destroyOwner(action) + return str(filename), '' + + with mock.patch.object( + importModule.QFileDialog, 'getOpenFileName', side_effect=importSelection + ): + with mock.patch.object(importModule, 'profileFromAny') as parse: + action.triggeredCallback(False) + parse.assert_not_called() + assert not isValid(action) + del action + + processQtEvents() + + assert len(destroyed) == iterations * 5 + collectAtBoundary() + assert all(reference() is None for reference in references) + assert not AppQDialog._openDialogs + + return {'modalPickerCycles': iterations, 'destroyed': len(destroyed), 'retained': 0} + + def main(): """Run the probe as a standalone source or Nuitka executable.""" parser = argparse.ArgumentParser() @@ -1855,6 +2030,8 @@ def main(): ) try: + print(json.dumps(runModalPickerProbe(arguments.iterations))) + print(json.dumps(runToolbarOwnershipProbe(arguments.iterations))) print(json.dumps(runNotificationAndDnsProbe(arguments.iterations))) print(json.dumps(runConnectionRecoveryProbe(arguments.iterations))) print(json.dumps(runReentrantLifetimeProbe(arguments.iterations))) diff --git a/tests/test_frozenlib.py b/tests/test_frozenlib.py index 102253a4..c552866a 100644 --- a/tests/test_frozenlib.py +++ b/tests/test_frozenlib.py @@ -60,6 +60,30 @@ class FrozenlibQtContextTest(unittest.TestCase): """Create the suite-owned QApplication before constructing widgets.""" application() + def testObjectValidityRequiresEveryNativeObjectToBeAlive(self): + """Check all arguments while retaining the single-object contract.""" + first = QtCore.QObject() + second = QtCore.QObject() + destroyed = QtCore.QObject() + deleteQObject(destroyed) + + try: + self.assertTrue(Mixins.qObjectIsValid(qobject=first)) + self.assertTrue(Mixins.qObjectIsValid(None)) + self.assertTrue(Mixins.qObjectIsValid(first, None, object(), second)) + self.assertFalse(Mixins.qObjectIsValid(destroyed)) + + for arguments in ( + (destroyed, first, second), + (first, destroyed, second), + (first, second, destroyed), + ): + with self.subTest(arguments=arguments): + self.assertFalse(Mixins.qObjectIsValid(*arguments)) + finally: + deleteQObject(first) + deleteQObject(second) + def testDisabledContextPreservesPriorAndNestedState(self): """Restore both initially enabled and initially disabled widgets.""" button = QPushButton() diff --git a/tests/test_qt_lifetime.py b/tests/test_qt_lifetime.py index bb86d9c6..14db3a82 100644 --- a/tests/test_qt_lifetime.py +++ b/tests/test_qt_lifetime.py @@ -19,11 +19,15 @@ from __future__ import annotations -from Furious.Backends.ExternalCore.Editor import ExternalCoreEditor +from Furious.Backends.ExternalCore.Editor import ( + ExternalCoreEditor, + ExternalCorePathInput, +) 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.AssetWindow import XrayAssetWindow from Furious.Backends.Xray.RoutingWindow import ( UserRoutingTableView, RoutingDocumentationURL, @@ -39,6 +43,7 @@ from Furious.Backends.Xray.VlessEditor import VlessEditor from Furious.Backends.Xray.VmessEditor import VmessEditor from Furious.Actions.Import import ( ImportURIsProgressDialog, + ImportFromFileAction, ImportQRCodeOnTheScreenAction, ) from Furious.Actions.Routing import RoutingAction @@ -60,6 +65,7 @@ from Furious.Qt import ( AppQSwitch, AppQSeparator, AppQTransientDialog, + AppQToolBar, connectWeakly, singleShotWeakly, ) @@ -88,14 +94,288 @@ from tests.support import ( from tests.fixtures.editor_lifetime_probe import runSingletonIPCProbe import gc +import importlib import builtins from pathlib import Path +from types import SimpleNamespace import tempfile import unittest from unittest import mock import weakref +class ToolbarLifetimeTest(unittest.TestCase): + """Constructor actions use the toolbar tree unless an owner was explicit.""" + + def testToolbarAdoptsOnlyUnownedActionsAndSeparators(self): + application() + + with isolatedSettings(): + for borrowed in (False, True): + with self.subTest(borrowed=borrowed): + owner = QWidget() + action = AppQAction( + 'Toolbar fixture', parent=owner if borrowed else None + ) + separator = AppQSeparator() + + if borrowed: + separator.setParent(owner) + toolbar = AppQToolBar(action, separator, parent=owner) + + try: + self.assertIs(action.parent(), owner if borrowed else toolbar) + self.assertIs( + separator.parent(), owner if borrowed else toolbar + ) + toolbar.actionTriggered.emit(action) + + deleteQObject(toolbar) + + self.assertEqual(isValid(action), borrowed) + self.assertEqual(isValid(separator), borrowed) + finally: + if isValid(owner): + deleteQObject(owner) + if isValid(action): + deleteQObject(action) + if isValid(separator): + deleteQObject(separator) + + +class ModalPickerLifetimeTest(unittest.TestCase): + """A nested chooser can destroy its caller before returning a selection.""" + + @staticmethod + def _destroyDuringModalLoop(owner): + loop = QtCore.QEventLoop() + + def finish(): + deleteQObject(owner) + loop.quit() + + QtCore.QTimer.singleShot(0, finish) + loop.exec() + + def testTextSaveDoesNotTruncateFileAfterWindowDestruction(self): + """Cancel before opening the destination when the editor no longer exists.""" + application() + module = importlib.import_module('Furious.Window.TextEditorWindow') + + with isolatedSettings(), tempfile.TemporaryDirectory() as directory: + filename = Path(directory) / 'existing.json' + filename.write_text('keep original bytes', encoding='utf-8') + parent = QWidget() + editor = TextEditorWindow(parent) + editor.jsonEditor.setPlainText('{"updated": true}') + + def select(*_args, **_kwargs): + self._destroyDuringModalLoop(parent) + return str(filename), '' + + with mock.patch.object( + module.QFileDialog, 'getSaveFileName', side_effect=select + ): + editor.saveAsFile() + + self.assertFalse(isValid(editor)) + self.assertEqual( + filename.read_text(encoding='utf-8'), 'keep original bytes' + ) + + def testCloseConfirmationStopsAfterSavingDestroysItsOwner(self): + """Do not close a deleted prompt or continue its former close event.""" + script = """ +import importlib +import sys +from unittest import mock +from PySide6 import QtCore +from PySide6.QtGui import QCloseEvent +from shiboken6 import delete as deleteQObject, isValid +from tests.support import application, isolatedSettings, processQtEvents +from Furious.Window.TextEditorWindow import TextEditorWindow + +application() +module = importlib.import_module('Furious.Window.TextEditorWindow') +errors = [] +sys.excepthook = lambda *args: errors.append(args) + +with isolatedSettings(): + editor = TextEditorWindow() + editor.modified = True + originalPrompt = module.MBoxQuestionSave + + class Prompt(originalPrompt): + def exec(self): + QtCore.QTimer.singleShot(0, self.button0.click) + return super().exec() + + def save(*args, **kwargs): + deleteQObject(editor) + return True + + with mock.patch.object(module, 'MBoxQuestionSave', Prompt): + with mock.patch.object(editor, 'save', side_effect=save): + editor.closeEvent(QCloseEvent()) + + processQtEvents() + assert not isValid(editor) + assert not errors, [(str(args[0]), str(args[1])) for args in errors] +""" + assertChildSucceeded( + self, runPythonChild(script), 'save destroys close-confirmation owner' + ) + + def testTextSaveCommitSurvivesDestructionDuringItsNotifications(self): + """A committed save must not touch the dead editor or open a stale notice.""" + application() + module = importlib.import_module('Furious.Window.TextEditorWindow') + + with isolatedSettings(): + for boundary in ('row-change', 'reconnect-notice'): + with self.subTest(boundary=boundary): + parent = QWidget() + editor = TextEditorWindow(parent) + editor.currentIndex = 0 + editor.jsonEditor.setPlainText('{"server": "after"}') + rows = [ + ServerProfile.fromConfiguration( + CoreConfiguration({'server': 'before'}) + ) + ] + + def flush(*_args): + if boundary == 'row-change': + deleteQObject(parent) + + def notice(*_args, **_kwargs): + if isValid(parent): + deleteQObject(parent) + + try: + with mock.patch.object( + module.Storage, 'UserServers', return_value=rows + ): + with mock.patch.object( + module.Storage, 'UserActivatedItemIndex', return_value=0 + ): + with mock.patch.object( + module, + 'AppMainWindow', + return_value=SimpleNamespace(flushRow=flush), + ): + with mock.patch.object( + module, + 'configurationFromMapping', + side_effect=CoreConfiguration, + ): + with mock.patch.object( + module, + 'showMBoxNewChangesNextTime', + side_effect=notice, + ) as showNotice: + self.assertTrue( + editor.save(showChangesMethod='exec') + ) + + if boundary == 'row-change': + showNotice.assert_not_called() + else: + showNotice.assert_called_once() + + self.assertFalse(isValid(editor)) + self.assertEqual(rows[0]['server'], 'after') + finally: + if isValid(parent): + deleteQObject(parent) + + def testPathBrowseStopsAfterItsFieldTreeIsDestroyed(self): + """A pure binding can survive after its native row widgets disappear.""" + application() + module = importlib.import_module('Furious.Backends.ExternalCore.Editor') + + with isolatedSettings(): + for directoryMode in (False, True): + with self.subTest(directory=directoryMode): + binding = ExternalCorePathInput( + 'Path', 'executable', directory=directoryMode + ) + container = binding._container + + def select(*_args, **_kwargs): + self._destroyDuringModalLoop(container) + return '/chosen' if directoryMode else ('/chosen', '') + + method = ( + 'getExistingDirectory' if directoryMode else 'getOpenFileName' + ) + try: + with mock.patch.object( + module.QFileDialog, method, side_effect=select + ): + binding.browse() + + self.assertFalse(isValid(binding._input)) + finally: + if isValid(binding._title): + deleteQObject(binding._title) + if isValid(container): + deleteQObject(container) + + def testAssetImportStopsAfterWindowDestruction(self): + """Do not pass a selected filename into a dead asset view.""" + application() + module = importlib.import_module('Furious.Backends.Xray.AssetWindow') + + with isolatedSettings(): + parent = QWidget() + window = XrayAssetWindow(parent) + + def select(*_args, **_kwargs): + self._destroyDuringModalLoop(parent) + return '/chosen.dat', '' + + with mock.patch.object( + module.QFileDialog, 'getOpenFileName', side_effect=select + ): + with mock.patch.object( + window.xrayAssetListView, 'appendNewItem' + ) as append: + window.appendNewItem() + append.assert_not_called() + + self.assertFalse(isValid(window)) + + def testFileImportStopsWhenItsActionIsDestroyed(self): + """A surviving main window must not receive work from a dead action.""" + application() + module = importlib.import_module('Furious.Actions.Import') + + with isolatedSettings(), tempfile.TemporaryDirectory() as directory: + filename = Path(directory) / 'input.json' + filename.write_text('{}', encoding='utf-8') + action = ImportFromFileAction() + mainWindow = mock.Mock() + + def select(*_args, **_kwargs): + self._destroyDuringModalLoop(action) + return str(filename), '' + + with mock.patch.object( + module.QFileDialog, 'getOpenFileName', side_effect=select + ): + with mock.patch.object( + module, 'AppMainWindow', return_value=mainWindow + ): + with mock.patch.object(module, 'profileFromAny') as parse: + with mock.patch.object(module, 'MBoxImportSuccess'): + action.triggeredCallback(False) + parse.assert_not_called() + mainWindow.appendNewItemByFactory.assert_not_called() + + self.assertFalse(isValid(action)) + + class ProbeTransientDialog(AppQTransientDialog): """Own a running timer and menu so their destruction can be observed."""