From 8a20f2eb29811a74eedc59f324bd08debd407dde Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Wed, 7 Oct 2026 16:20:04 +0800 Subject: [PATCH] Fix window presentation teardown Signed-off-by: Loren Eteval --- Furious/Qt/AGENTS.md | 6 + Furious/Qt/QtWidgets.py | 2 +- Furious/Window/QRCodeWindow.py | 6 +- tests/README.md | 3 + tests/fixtures/editor_lifetime_probe.py | 198 +++++++++++++++++++++++- tests/test_main_window_geometry.py | 22 +++ tests/test_qr_export_scalability.py | 44 +++++- 7 files changed, 277 insertions(+), 4 deletions(-) diff --git a/Furious/Qt/AGENTS.md b/Furious/Qt/AGENTS.md index c7422e89..f8139fe9 100644 --- a/Furious/Qt/AGENTS.md +++ b/Furious/Qt/AGENTS.md @@ -96,6 +96,9 @@ behavior, and lifetime primitives; pages and services consume them without creat strongly. User hooks and signal delivery can synchronously destroy the reply or manager; recheck native validity before subsequent hooks or Qt cleanup. `test_service_runtime.py` covers completion and abort reentrancy. Do not attach ad-hoc attributes to third-party Qt objects or multiply timers/connections across show/hide cycles. + 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. - 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 @@ -107,6 +110,9 @@ behavior, and lifetime primitives; pages and services consume them without creat - Top-level windows use canonical first-show preparation. Save geometry/state only after a native presentation; a never-shown Qt fallback must not overwrite persisted user geometry. Do not call overridable geometry hooks from constructors or manipulate private first-show state. + The shared post-show event flush can run cancellation or native destruction before returning. Callers recheck + validity and operation state before activating a window or starting further work. The show-flush cases in + `tests/test_main_window_geometry.py` and `tests/test_qr_export_scalability.py` cover those continuation boundaries. - A stylesheet border radius paints a rounded frame but does not clip child viewports or table headers. Padding can protect the corners while introducing a visible inset; assess both effects before changing shared view styles. Popup native-window transparency is a separate boundary from in-window child painting. diff --git a/Furious/Qt/QtWidgets.py b/Furious/Qt/QtWidgets.py index bc6147e9..354e47a5 100644 --- a/Furious/Qt/QtWidgets.py +++ b/Furious/Qt/QtWidgets.py @@ -915,7 +915,7 @@ class AppQMainWindow( # initial geometry and centering above do not depend on this event pump. APP().processEvents() - if PLATFORM == 'Darwin': + if PLATFORM == 'Darwin' and isValid(self): self.activateWindow() self.raise_() diff --git a/Furious/Window/QRCodeWindow.py b/Furious/Window/QRCodeWindow.py index f02c2b5a..1c9bda21 100644 --- a/Furious/Window/QRCodeWindow.py +++ b/Furious/Window/QRCodeWindow.py @@ -34,6 +34,8 @@ from PySide6 import QtCore from PySide6.QtGui import QImage, QPixmap from PySide6.QtWidgets import QLabel, QSizePolicy, QVBoxLayout, QWidget +from shiboken6 import isValid + import segno import logging @@ -386,7 +388,9 @@ class QRCodeWindow(AppQMainWindow): self._exporting = True self.show() - self._exportTimer.start(0) + + if isValid(self) and self._exporting: + self._exportTimer.start(0) return self diff --git a/tests/README.md b/tests/README.md index 96720276..fa1634ff 100644 --- a/tests/README.md +++ b/tests/README.md @@ -343,6 +343,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 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. Endpoint lookup covers service destruction/disablement during state and result notifications, without admitting the next request from the abandoned stage. diff --git a/tests/fixtures/editor_lifetime_probe.py b/tests/fixtures/editor_lifetime_probe.py index b3bb65c5..69d76f9a 100644 --- a/tests/fixtures/editor_lifetime_probe.py +++ b/tests/fixtures/editor_lifetime_probe.py @@ -57,6 +57,7 @@ from Furious.Service.ProfileTesting import ( _DownloadSpeedWorker, ) from Furious.Service.ConnectivityManager import ConnectivityManager +from Furious.Service.UpdateManager import UpdateManager from Furious.Service.DnsResolver import DnsResolutionOperation, DnsResolver from Furious.Service.TrafficStatsManager import TrafficStatsManager from Furious.Frozenlib import Mixins @@ -69,13 +70,15 @@ from Furious.Models import CoreConfiguration from Furious.Controllers.SettingsController import SettingsController 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.Widget.ServerTableView import ServerTableView import PySide6 from PySide6 import QtCore from PySide6.QtNetwork import QLocalSocket, QNetworkReply -from PySide6.QtWidgets import QPushButton, QWidget +from PySide6.QtWidgets import QPushButton, QWidget, QMainWindow from shiboken6 import isValid, delete as deleteQObject @@ -1009,6 +1012,194 @@ def runServiceTeardownProbe(iterations=100): } +def runPublicationTeardownProbe(iterations=100): + """Verify callback and event-flush teardown in native and compiled Qt.""" + application() + httpCompletions = 0 + + for once in (True, False): + for _ in range(iterations): + manager = HttpGetManager(completionRunsOnce=once) + replies = [_PendingReply(manager), _PendingReply(manager)] + resources = { + key: QtCore.QTimer(manager) + for key in (('shared',) if once else ('first', 'second')) + } + completed = [] + + def complete(**context): + marker = context['marker'] + completed.append(marker) + resource = resources['shared' if once else marker] + resource.stop() + deleteQObject(resource) + + if marker == 'first': + replies[1].finished.emit() + + manager.completionCallback = complete + + with mock.patch.object(manager, 'get', side_effect=replies): + manager.webGET('https://invalid.test/first', marker='first') + manager.webGET('https://invalid.test/second', marker='second') + + replies[0].finished.emit() + processQtEvents() + + assert completed == (['first'] if once else ['first', 'second']) + assert not manager._replyContexts + assert all(not isValid(reply) for reply in replies) + assert all(not isValid(resource) for resource in resources.values()) + httpCompletions += len(completed) + deleteQObject(manager) + + windowsDestroyed = 0 + + for _ in range(iterations): + window = AppQMainWindow() + before = set(AppQMainWindow._openWindows) + QtCore.QTimer.singleShot(0, lambda: deleteQObject(window)) + + with mock.patch('Furious.Qt.QtWidgets.PLATFORM', 'Darwin'): + window.show() + + assert not isValid(window) + assert set(AppQMainWindow._openWindows) == before + windowsDestroyed += 1 + + profile = SimpleNamespace() + qrPresentations = 0 + + for destroy in (False, True): + for _ in range(iterations): + window = QRCodeWindow() + reference = weakref.ref(window) + + def endExport(): + if destroy: + deleteQObject(window) + else: + window.cancelExport() + + QtCore.QTimer.singleShot(0, endExport) + + with mock.patch( + 'Furious.Window.QRCodeWindow.captureQRCodeExportItems', + return_value=(profile, profile), + ): + window.startExportByIndex([0, 1]) + + if destroy: + assert not isValid(window) and not isValid(window._exportTimer) + else: + assert not window.isExporting() and not window._exportTimer.isActive() + window.close() + + processQtEvents() + del window + + assert reference() is None + qrPresentations += 1 + + response = SimpleNamespace( + readAll=lambda: QtCore.QByteArray( + json.dumps( + { + 'tag_name': '999.0.0', + 'html_url': 'https://github.com/LorenEteval/Furious/releases', + } + ).encode() + ) + ) + + for target in ('manager', 'parent'): + for _ in range(iterations): + manager = UpdateManager() + parent = QWidget() + before = set(AppQDialog._openDialogs) + + def notified(_version): + deleteQObject(manager if target == 'manager' else parent) + + manager.successCallback( + response, parent=parent, hasNewVersionCallback=notified + ) + + assert set(AppQDialog._openDialogs) == before + + if isValid(manager): + deleteQObject(manager) + + if isValid(parent): + deleteQObject(parent) + + # Home badges/services are persistent application receivers. Exercise each + # native teardown boundary once; their direct connections are not transient. + controller = ConnectionController(coreManager=SimpleNamespace(runtimes=[])) + + with mock.patch( + 'Furious.Window.HomePage.AppConnectionController', return_value=controller + ): + parent = QWidget() + badge = NetworkStateBadge(parent) + badge.layoutRequirementChanged.connect(lambda: deleteQObject(parent)) + badge.setStatus('success', 'fixture') + assert not isValid(badge) + + deleteQObject(controller) + + class StatusOwner(HomePage): + statusPublished = QtCore.Signal() + + def __init__(self): + QMainWindow.__init__(self) + + def setNetworkState(self, _success, **_kwargs): + self.statusPublished.emit() + + def resetNetworkState(self): + self.statusPublished.emit() + + for boundary in ('success', 'failure', 'start', 'disconnect'): + parent = StatusOwner() + manager = AppConnectivityManager(parent) + reply = _PendingReply(manager) + parent.statusPublished.connect(lambda: deleteQObject(parent)) + + if boundary in ('success', 'failure'): + with mock.patch.object(manager, 'get', lambda _request: reply): + manager.webGET('https://invalid.test') + + if boundary == 'failure': + reply.setError( + QNetworkReply.NetworkError.UnknownNetworkError, 'fixture' + ) + + reply.finished.emit() + elif boundary == 'start': + with mock.patch( + 'Furious.Window.HomePage.AppConnectionController', + return_value=SimpleNamespace(isConnected=lambda: False), + ): + manager.jobArrangeTimer.timeout.emit() + else: + manager.disconnectedCallback() + + assert not isValid(manager) and not isValid(reply) + assert not isValid(manager.jobTimeoutTimer) + assert not isValid(manager.jobArrangeTimer) + + processQtEvents() + + return { + 'httpTerminalCompletions': httpCompletions, + 'showFlushWindowsDestroyed': windowsDestroyed, + 'qrPresentationsEnded': qrPresentations, + 'updateOwnershipBoundaries': iterations * 2, + 'homeStatusDestructionBoundaries': 5, + } + + def runNetworkProbe(iterations=100): """Verify native teardown releases both reply registries under compilation.""" application() @@ -1665,6 +1856,11 @@ def main(): print(json.dumps(runConfirmationProbe(arguments.iterations), sort_keys=True)) print(json.dumps(runNetworkProbe(arguments.iterations), sort_keys=True)) print(json.dumps(runServiceTeardownProbe(arguments.iterations), sort_keys=True)) + print( + json.dumps( + runPublicationTeardownProbe(arguments.iterations), sort_keys=True + ) + ) print(json.dumps(runInfrastructureProbe(arguments.iterations), sort_keys=True)) print( diff --git a/tests/test_main_window_geometry.py b/tests/test_main_window_geometry.py index 4c414d5e..59bb8a26 100644 --- a/tests/test_main_window_geometry.py +++ b/tests/test_main_window_geometry.py @@ -37,6 +37,8 @@ from Furious.Window.TextEditorWindow import TextEditorWindow from PySide6 import QtCore from PySide6.QtWidgets import QMainWindow, QWidget +from shiboken6 import isValid, delete as deleteQObject + from tests.support import ( application, collectAtBoundary, @@ -175,6 +177,26 @@ class AppQMainWindowLifecycleTest(unittest.TestCase): collectAtBoundary() + def testEventFlushCanDestroyTheWindowBeforeMacActivation(self): + """The synchronous show flush can end native ownership before activation.""" + for _ in range(30): + window = _LifecycleWindow() + baseline = set(AppQMainWindow._openWindows) + destroyed = [] + window.destroyed.connect(lambda *_args: destroyed.append(True)) + QtCore.QTimer.singleShot(0, lambda: deleteQObject(window)) + + try: + with patch('Furious.Qt.QtWidgets.PLATFORM', 'Darwin'): + window.show() + + self.assertEqual(destroyed, [True]) + self.assertFalse(isValid(window)) + self.assertEqual(set(AppQMainWindow._openWindows), baseline) + finally: + if isValid(window): + deleteQObject(window) + def testPreparationRunsAfterCompositionAndOnlyOnce(self): """Never call subclass lifecycle hooks from the base constructor.""" window = _LifecycleWindow() diff --git a/tests/test_qr_export_scalability.py b/tests/test_qr_export_scalability.py index 86a79f33..fbf83cce 100644 --- a/tests/test_qr_export_scalability.py +++ b/tests/test_qr_export_scalability.py @@ -29,7 +29,7 @@ from Furious.Window.QRCodeWindow import ( from PySide6 import QtCore from PySide6.QtGui import QImage -from shiboken6 import isValid +from shiboken6 import isValid, delete as deleteQObject from unittest import mock @@ -116,6 +116,48 @@ class QRCodeExportScalabilityTest(unittest.TestCase): ) ) + def testPresentationFlushCanDestroyOrCancelTheIncrementalExport(self): + """Do not start a timer after show delivery ends the export's ownership.""" + profiles = self.profiles(2) + + for destroy in (False, True): + with self.subTest(destroy=destroy): + for _ in range(20): + window = QRCodeWindow() + reference = weakref.ref(window) + + def endExport(): + if destroy: + deleteQObject(window) + else: + window.cancelExport() + + QtCore.QTimer.singleShot(0, endExport) + + try: + with mock.patch( + 'Furious.Window.QRCodeWindow.Storage.UserServers', + return_value=profiles, + ): + window.startExportByIndex([0, 1]) + + if destroy: + self.assertFalse(isValid(window)) + self.assertFalse(isValid(window._exportTimer)) + else: + self.assertFalse(window.isExporting()) + self.assertFalse(window._exportTimer.isActive()) + self.assertEqual(window.exportProcessedCount(), 0) + finally: + if isValid(window): + window.close() + + processQtEvents() + + del window + + self.assertIsNone(reference()) + def testSingleProfileExportRemainsImmediate(self): """Keep one-profile export synchronous without scheduling a batch.""" profiles = self.profiles(1)