From 47ffb8024d1a507ad632dac630c52e4ac8e87e5e Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Tue, 6 Oct 2026 12:12:47 +0800 Subject: [PATCH] Fix Qt callback lifetimes Signed-off-by: Loren Eteval --- Furious/Application/AGENTS.md | 5 + Furious/Application/DesktopApplication.py | 26 +- Furious/Window/SettingsPage.py | 5 +- tests/README.md | 14 +- tests/fixtures/editor_lifetime_probe.py | 276 +++++++++++++++++++++- tests/test_qt_lifetime.py | 7 + tests/test_sing_tun.py | 37 +++ 7 files changed, 352 insertions(+), 18 deletions(-) diff --git a/Furious/Application/AGENTS.md b/Furious/Application/AGENTS.md index 914b672..fc6a635 100644 --- a/Furious/Application/AGENTS.md +++ b/Furious/Application/AGENTS.md @@ -30,6 +30,11 @@ boundary between the outer child-process supervisor and the inner application ev endpoint, and fails closed when ownership is uncertain, including privilege handoff. A successful Windows local-server listen alone does not establish exclusivity; command delivery and endpoint ownership are separate observations. +- Each singleton IPC connection creates a short-lived socket sender. Use weak named dispatch with sender forwarding + to the application; repeatedly connecting a compiled application bound method can grow Nuitka's protection list + even after the native sockets die. The server owns sockets through their one-command completion/disconnection. + Tray actions have an explicit QObject owner, while the borrowed top-level tray menu needs a native destruction + boundary independent of the tray wrapper's Python lifetime. The lifetime probe covers both contracts. - Native session callbacks cross to the GUI thread before touching Qt-owned state. Tray, dock, System Proxy daemon, Flatpak/AppImage, and no-tray behavior are explicit platform capabilities. - The application owns the top-level window/tray wrappers; `MainWindow` owns the persistent page tree. Cleanup order diff --git a/Furious/Application/DesktopApplication.py b/Furious/Application/DesktopApplication.py index 7908aff..0244b28 100644 --- a/Furious/Application/DesktopApplication.py +++ b/Furious/Application/DesktopApplication.py @@ -58,7 +58,7 @@ from Furious.Controllers import ( ) from Furious.Extensions import BUNDLED_EXTENSION_TYPES from Furious.Plugins import initializePluginRegistry -from Furious.Qt import AppQMessageBox, AppStyleSheet, ThemeTransition +from Furious.Qt import AppQMessageBox, AppStyleSheet, ThemeTransition, connectWeakly from Furious.Qt.TextEditorTheme import configureEditorLogMetadata from Furious.Qt import gettext as _ from Furious.Repository import Storage @@ -72,13 +72,13 @@ from PySide6.QtGui import QFontDatabase, QPalette from PySide6.QtNetwork import QLocalServer, QLocalSocket from PySide6.QtWidgets import QApplication +from enum import Enum + import os import sys import logging import platform import traceback -from enum import Enum - import darkdetect logger = logging.getLogger(__name__) @@ -471,17 +471,21 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): if socket is None: continue - # QLocalServer owns pending sockets until they are explicitly - # released. Use sender() instead of a partial that retains each - # socket and dispose it after the one-command protocol completes. - socket.readyRead.connect(self.handleNewData) + # Every launch creates a new sender. Weak dispatch avoids adding a + # protected compiled application method for each completed socket. + connectWeakly( + socket.readyRead, + self, + 'handleNewData', + sender=socket, + forwardSender=True, + ) + socket.disconnected.connect(socket.deleteLater) - @QtCore.Slot() - def handleNewData(self): + @QtCore.Slot(QtCore.QObject) + def handleNewData(self, socket): """Handle new data.""" - socket = self.sender() - if not isinstance(socket, QLocalSocket): return diff --git a/Furious/Window/SettingsPage.py b/Furious/Window/SettingsPage.py index 5efd93d..571a6ab 100644 --- a/Furious/Window/SettingsPage.py +++ b/Furious/Window/SettingsPage.py @@ -577,7 +577,10 @@ class _TUNBackendSettingsCard(_SettingsCard): self.sync() connectWeakly(self.comboBox.currentIndexChanged, self, '_selectionChanged') - connectWeakly(AppSettingsController().tunBackendChanged, self, 'sync') + + controller = AppSettingsController() + + connectWeakly(controller.tunBackendChanged, self, 'sync', sender=controller) def sync(self, backend=None): blocker = QtCore.QSignalBlocker(self.comboBox) diff --git a/tests/README.md b/tests/README.md index cc80e94..d908e1f 100644 --- a/tests/README.md +++ b/tests/README.md @@ -326,15 +326,21 @@ python -m tests.fixtures.editor_lifetime_probe --iterations 100 --pattern repres Patterns also include `alternating`, `reverse`, `hysteria2`, and `vless`. The representative pattern cycles through Hysteria2, VLESS, VMess, Trojan, SOCKS, Hysteria1, and External Core. Each invocation additionally probes HTTP completion -and owner/reply-first destruction, button ownership, signal endpoints, masks, -reopen generations, animations, menus, and view-owned confirmations. It records +and owner/reply-first destruction, subscription tracking, singleton IPC callbacks, +settings-controller receivers, tray/actions/progress teardown, TCPing owner-first +shutdown and engine destruction affinity, button ownership, signal endpoints, +masks, reopen generations, animations, menus, and view-owned confirmations. It records JSON diagnostics and asserts captured Qt callback exceptions are absent. For compiler-sensitive work, compile this fixture separately with Nuitka's PySide6 plugin, its imported support code, and required data, then repeat the close/accept/reject checks. Record the Python/PySide6/Nuitka versions, target, and -build flags. A missing private protected-method counter is unknown, not zero; -combine available diagnostics with native destruction, weak-wrapper, and registry +build flags. Include the harness configuration with +`--include-data-files=tests/fixtures/offscreen.json=tests/fixtures/offscreen.json` +and Furious package data. The private callback counter is toolchain-specific; a +missing counter is unknown, not zero. The IPC probe includes a bounded direct +connection control to validate an available counter. Combine available +diagnostics with native destruction, weak-wrapper, and registry evidence. A diagnostic build passing does not establish that an ordinary release build has the same behavior. diff --git a/tests/fixtures/editor_lifetime_probe.py b/tests/fixtures/editor_lifetime_probe.py index a56756d..d8dcbf3 100644 --- a/tests/fixtures/editor_lifetime_probe.py +++ b/tests/fixtures/editor_lifetime_probe.py @@ -23,6 +23,12 @@ from Furious.Backends import OFFICIAL_PLUGIN_TYPES from Furious.Backends.Xray.RoutingWindow import RoutingRulesDialog from Furious.Backends.Xray.AssetListView import XrayAssetListView import Furious.Backends.Xray.AssetListView as assetModule +import Furious.Actions.Import as importModule +from Furious.Actions.Routing import RoutingAction +from Furious.Application.TrayIcon import TrayIcon +from Furious.Application.DesktopApplication import DesktopApplication +from Furious.Controllers import ConnectionController, RoutingController +from Furious.Plugins import RoutingOption from Furious.Plugins import blankProfile, initializePluginRegistry from Furious.Qt import ( AppQAction, @@ -34,12 +40,17 @@ from Furious.Qt import ( ) from Furious.Qt.HttpGetManager import HttpGetManager from Furious.Service.EndpointInfoService import ProxyEndpointHttpClient +from Furious.Service.SubscriptionManager import SubscriptionManager +from Furious.Service.ProfileTesting import _LatencyScheduler +from Furious.Controllers.SettingsController import SettingsController +from Furious.Repository import Storage +from Furious.Window.SettingsPage import _TUNBackendSettingsCard from Furious.Widget.ServerTableView import ServerTableView import PySide6 from PySide6 import QtCore -from PySide6.QtNetwork import QNetworkReply +from PySide6.QtNetwork import QLocalSocket, QNetworkReply from PySide6.QtWidgets import QPushButton, QWidget from shiboken6 import isValid, delete as deleteQObject @@ -49,9 +60,11 @@ from tests.support import ( collectAtBoundary, isolatedSettings, processQtEvents, + waitFor, ) from collections import Counter +from types import SimpleNamespace import argparse import json @@ -78,6 +91,74 @@ PROTOCOL_PATTERNS = { CLOSE_METHODS = ('accept', 'close', 'reject') +class _SingletonReceiver(QtCore.QObject): + """Exercise the real IPC methods without application startup or endpoints.""" + + handleNewData = DesktopApplication.handleNewData + + def __init__(self): + super().__init__() + self.pending = [] + self.systemTray = None + self.server = SimpleNamespace( + hasPendingConnections=lambda _pending=self.pending: bool(_pending), + nextPendingConnection=lambda _pending=self.pending: _pending.pop(0), + ) + + +def runSingletonIPCProbe(iterations: int = 100) -> dict[str, object]: + """Destroy completed IPC sockets and bound compiled callback growth.""" + application() + protectedMethods = getattr( + sys.modules.get('PySide6-postLoad', PySide6), '_protected', None + ) + directGrowth = None + + if protectedMethods is not None: + # Positive control: native sender destruction does not retire this + # toolchain's protected compiled bound method. Keep the diagnostic bounded. + control = _SingletonReceiver() + socket = QLocalSocket(control) + before = len(protectedMethods) + socket.readyRead.connect(control.handleNewData) + deleteQObject(control) + directGrowth = len(protectedMethods) - before + assert directGrowth == 1, directGrowth + del socket, control + + receiver = _SingletonReceiver() + receiverReference = weakref.ref(receiver) + before = len(protectedMethods) if protectedMethods is not None else None + references = [] + destroyed = [] + + for _ in range(iterations): + socket = QLocalSocket(receiver) + references.append(weakref.ref(socket)) + socket.destroyed.connect(lambda *_args: destroyed.append(True)) + receiver.pending.append(socket) + # Bypass the launch rate limit, preserving the actual connection logic. + DesktopApplication.handleNewConnection.__wrapped__(receiver) + socket.readyRead.emit() + processQtEvents() + assert not isValid(socket) + del socket + + growth = len(protectedMethods) - before if before is not None else None + assert growth in (None, 0), growth + assert len(destroyed) == iterations + assert all(reference() is None for reference in references) + deleteQObject(receiver) + del receiver + assert receiverReference() is None + + return { + 'singletonSocketsDestroyed': len(destroyed), + 'singletonProtectedMethodGrowth': growth, + 'directConnectionControlGrowth': directGrowth, + } + + def runProbe( iterations: int = 100, *, @@ -109,7 +190,11 @@ def runProbe( importActionsFactory=tuple, ) - protectedMethods = getattr(PySide6, '_protected', None) + # Nuitka 4.2.1 keeps this private list in its synthetic post-load module. + # Other toolchains may expose it on PySide6 or not expose it at all. + protectedMethods = getattr( + sys.modules.get('PySide6-postLoad', PySide6), '_protected', None + ) protectedMethodsBefore = ( len(protectedMethods) if isinstance(protectedMethods, list) else None ) @@ -347,6 +432,189 @@ def runNetworkProbe(iterations=100): return result +def runSettingsAndSubscriptionProbe(iterations=100): + """Verify independent controller edges and early subscription reply deletion.""" + app = application() + previousController = app.settingsController + controller = SettingsController() + app.settingsController = controller + references = [] + signal = QtCore.SIGNAL('tunBackendChanged(QString)') + baseline = controller.receivers(signal) + subsGetter = Storage.UserSubs + Storage.UserSubs = staticmethod(dict) + manager = SubscriptionManager() + + try: + with isolatedSettings(): + for index in range(iterations): + card = _TUNBackendSettingsCard() + references.append(weakref.ref(card)) + controller.tunBackendChanged.emit('tun2socks') + assert card.comboBox.currentData() == 'tun2socks' + assert controller.receivers(signal) == baseline + 1 + + deleteQObject(card) + del card + assert controller.receivers(signal) == baseline + controller.tunBackendChanged.emit('sing-tun') + + reply = _PendingReply(manager) + references.append(weakref.ref(reply)) + manager.get = lambda _request: reply + manager.updateSubsByWebGET( + webURL='https://invalid.test', unique=str(index) + ) + del manager.get + deleteQObject(reply) + del reply + + assert not manager._replyContexts + assert not manager._activeReplies + assert not manager._replySubscriptions + manager.cancelUpdates() + + replies = [_PendingReply(manager), _PendingReply(manager)] + for index, reply in enumerate(replies): + manager.get = lambda _request: reply + manager.updateSubsByWebGET( + webURL='https://invalid.test', unique=str(index) + ) + del manager.get + + replies[0].abort = lambda: deleteQObject(manager) + manager.cancelUpdates() + assert not isValid(manager) + assert all(not isValid(reply) for reply in replies) + assert not manager._activeReplies and not manager._replySubscriptions + + deleteQObject(controller) + collectAtBoundary() + assert all(reference() is None for reference in references) + finally: + app.settingsController = previousController + Storage.UserSubs = staticmethod(subsGetter) + if isValid(controller): + deleteQObject(controller) + if isValid(manager): + manager.shutdown() + deleteQObject(manager) + + return {'selectorCycles': iterations, 'subscriptionReplyCycles': iterations} + + +class _CaptureHandle: + """Stand in for desktop capture without acquiring a host handle.""" + + def __init__(self): + self.closeCount = 0 + + def close(self): + self.closeCount += 1 + + +def runTrayOwnershipProbe(iterations=100): + """Prove native teardown even if compiled callbacks retain Python wrappers.""" + app = application() + previousConnection, previousRouting = ( + app.connectionController, + app.routingController, + ) + captureFactory = importModule.mss.mss + importModule.mss.mss = _CaptureHandle + connection, routing = ConnectionController(), RoutingController() + app.connectionController, app.routingController = connection, routing + options = (RoutingOption('one', 'One'), RoutingOption('two', 'Two')) + + try: + for _ in range(iterations): + tray = TrayIcon() + action = tray.ConnectAction + progress = action.progressWidget + captureAction = next( + child + for child in tray.ImportAction._menu._actions + if isinstance(child, importModule.ImportQRCodeOnTheScreenAction) + ) + capture = captureAction.sct + progress.start(50) + + tray.RoutingAction._applyState(options, 'one') + retired = tuple(tray.RoutingAction._menu.actions()) + tray.RoutingAction._applyState(options, 'two') + processQtEvents() + assert all(not isValid(child) for child in retired) + current = tuple(tray.RoutingAction._menu.actions()) + resources = ( + tray._menu, + action, + tray.ImportAction, + tray.ImportAction._menu, + captureAction, + progress, + progress._widget.timer, + *current, + ) + + deleteQObject(tray) + processQtEvents() + assert all(not isValid(ob) for ob in resources) + assert capture.closeCount == 1 and captureAction.sct is None + + finally: + app.connectionController, app.routingController = ( + previousConnection, + previousRouting, + ) + importModule.mss.mss = captureFactory + connection.shutdown() + deleteQObject(connection) + deleteQObject(routing) + + return {'trayOwnershipCycles': iterations, 'routingRebuildCycles': iterations} + + +def runThreadOwnershipProbe(iterations=100): + """Stop the native thread before Qt deletes a scheduler or its parent.""" + application() + destroyed = [] + references = [] + + for parentFirst in (False, True): + for _ in range(iterations): + owner = QtCore.QObject() + scheduler = _LatencyScheduler( + lambda target: None, + lambda target, result: False, + pingConcurrency=1, + tcpingConcurrency=1, + parent=owner, + ) + engine = scheduler.ensureTcpingEngine() + thread = scheduler.tcpingThread + references.extend((weakref.ref(engine), weakref.ref(thread))) + engine.destroyed.connect( + lambda *_args: destroyed.append( + QtCore.QThread.currentThread() is thread + ), + QtCore.Qt.ConnectionType.DirectConnection, + ) + assert waitFor(thread.isRunning) + deleteQObject(owner if parentFirst else scheduler) + assert all(not isValid(ob) for ob in (scheduler, thread, engine)) + if isValid(owner): + deleteQObject(owner) + del engine, thread, scheduler, owner + processQtEvents() + + assert len(destroyed) == iterations * 2 and all(destroyed) + assert all(reference() is None for reference in references) + return { + 'threadOwnerFirstCycles': iterations * 2, + 'destroyedInWorkerThread': len(destroyed), + } + + def runButtonOwnershipProbe(iterations=100): """Exercise button detach/reuse, native removal, and owner-first callbacks.""" application() @@ -703,6 +971,10 @@ def main(): ) try: + print(json.dumps(runSingletonIPCProbe(arguments.iterations))) + print(json.dumps(runThreadOwnershipProbe(arguments.iterations))) + print(json.dumps(runTrayOwnershipProbe(arguments.iterations))) + print(json.dumps(runSettingsAndSubscriptionProbe(arguments.iterations))) print(json.dumps(runButtonOwnershipProbe(arguments.iterations), sort_keys=True)) print(json.dumps(runConfirmationProbe(arguments.iterations), sort_keys=True)) print(json.dumps(runNetworkProbe(arguments.iterations), sort_keys=True)) diff --git a/tests/test_qt_lifetime.py b/tests/test_qt_lifetime.py index 4516eb2..00297cd 100644 --- a/tests/test_qt_lifetime.py +++ b/tests/test_qt_lifetime.py @@ -84,6 +84,7 @@ from tests.support import ( runPythonChild, waitFor, ) +from tests.fixtures.editor_lifetime_probe import runSingletonIPCProbe import gc import builtins @@ -158,6 +159,12 @@ class DelayedReceiver(QtCore.QObject): class QtLifetimeTest(unittest.TestCase): """Stress direct destruction evidence without relying on process RSS alone.""" + def testSingletonSocketsReleaseTheirCallbackAndNativeOwner(self): + """Repeated IPC registration must release completed socket wrappers.""" + result = runSingletonIPCProbe(30) + self.assertEqual(result['singletonSocketsDestroyed'], 30) + self.assertIn(result['singletonProtectedMethodGrowth'], (None, 0)) + def testMenuOwnsOnlyPreviouslyUnparentedConstructorActions(self): """Native menu deletion releases owned actions even with held wrappers.""" application() diff --git a/tests/test_sing_tun.py b/tests/test_sing_tun.py index df6c6c4..970a5c7 100644 --- a/tests/test_sing_tun.py +++ b/tests/test_sing_tun.py @@ -70,6 +70,7 @@ import threading import tempfile import unittest import ipaddress +import weakref class _Engine: @@ -422,6 +423,42 @@ class SingTUNUIAndStorageTest(unittest.TestCase): def setUpClass(cls): cls.app = application() + def testSelectorDestructionDisconnectsTheIndependentController(self): + """Repeated cards release dispatchers while the controller remains alive.""" + module = importlib.import_module('Furious.Window.SettingsPage') + + with isolatedSettings(): + controller = SettingsController() + signal = QtCore.SIGNAL('tunBackendChanged(QString)') + baseline = controller.receivers(signal) + + with mock.patch.object( + module, 'AppSettingsController', return_value=controller + ): + for _ in range(20): + card = module._TUNBackendSettingsCard() + reference = weakref.ref(card) + controller.tunBackendChanged.emit('tun2socks') + self.assertEqual(card.comboBox.currentData(), 'tun2socks') + self.assertEqual(controller.receivers(signal), baseline + 1) + + card.deleteLater() + processQtEvents() + self.assertFalse(isValid(card)) + del card + + self.assertIsNone(reference()) + self.assertEqual(controller.receivers(signal), baseline) + controller.tunBackendChanged.emit('sing-tun') + + card = module._TUNBackendSettingsCard() + controller.deleteLater() + processQtEvents() + self.assertTrue(isValid(card)) + card.deleteLater() + processQtEvents() + self.assertFalse(isValid(card)) + def testTwoChoiceSelectorRetranslatesWithoutChangingStablePreference(self): module = importlib.import_module('Furious.Window.SettingsPage')