Harden Qt callbacks and subscription diagnostics

Signed-off-by: Loren Eteval <loren.eteval@proton.me>
This commit is contained in:
Loren Eteval
2026-08-27 16:43:01 +08:00
parent 16637ec545
commit dd185d3a20
13 changed files with 308 additions and 63 deletions
+2 -2
View File
@@ -6,8 +6,8 @@
present the outcome. They do not become authorities for connection, routing, subscription, or persistence state.
- Some existing import actions still perform parsing, repository insertion, screen capture, and a cooperative batch
dialog directly. Treat that as a compatibility path, not a template: new reusable or fallible workflows belong in an
injected service/controller and may be migrated there without preserving action-local orchestration. If touching its
timer-driven dialog, also remove transient bound-method `QTimer.singleShot()` callbacks per `Furious/Qt/AGENTS.md`.
injected service/controller and may be migrated there without preserving action-local orchestration. Its timer-driven
dialog yields through weak named-method scheduling; preserve that packaged-lifetime boundary per `Furious/Qt/AGENTS.md`.
- Share one `QAction` command between menus/buttons when they represent the same operation so callback, enabled/check
state, shortcut, and translation cannot diverge. `AppQAction.callback` is a strong reference; the action owner must not
outlive a captured receiver, and dynamic menus must release obsolete actions and callbacks.
+11 -4
View File
@@ -24,6 +24,7 @@ from Furious.Models import *
from Furious.Plugins import profileFromAny
from Furious.Repository import *
from Furious.Qt import *
from Furious.Qt.Signals import connectWeakly, singleShotWeakly
from Furious.Qt import gettext as _
from Furious.Widget.WaitingSpinner import *
@@ -169,7 +170,13 @@ class ImportURIsProgressDialog(AppQTransientDialog):
self.detailLabel = AppQLabel()
self.detailLabel.setWordWrap(True)
self.cancelButton = AppQPushButton(_('Cancel'))
self.cancelButton.clicked.connect(self.cancel)
connectWeakly(
self.cancelButton.clicked,
self,
'cancel',
sender=self.cancelButton,
)
statusLayout = QHBoxLayout()
statusLayout.addWidget(self.spinner)
@@ -190,7 +197,7 @@ class ImportURIsProgressDialog(AppQTransientDialog):
self.spinner.start()
QtCore.QTimer.singleShot(0, self.importNext)
singleShotWeakly(0, self, 'importNext')
return result
@@ -198,7 +205,7 @@ class ImportURIsProgressDialog(AppQTransientDialog):
"""Reject the current import ur is progress dialog values."""
self.cancel()
def cancel(self):
def cancel(self, *_args):
"""Cancel the import ur is progress dialog operation."""
self.canceled = True
self.cancelButton.setEnabled(False)
@@ -254,7 +261,7 @@ class ImportURIsProgressDialog(AppQTransientDialog):
self.updateStatus()
QtCore.QTimer.singleShot(0, self.importNext)
singleShotWeakly(0, self, 'importNext')
def finishImport(self):
"""Handle finish import for the import ur is progress dialog."""
+8 -1
View File
@@ -1179,7 +1179,14 @@ class UserRoutingTableView(Mixins.QTranslatable, AppQTableView):
routing = self.sourceModel.routingByRow(indexes[0])
dialog = RoutingRulesDialog(routing, parent=self)
dialog.finished.connect(self._rulesDialogFinished)
connectWeakly(
dialog.finished,
self,
'_rulesDialogFinished',
sender=dialog,
)
dialog.open()
@QtCore.Slot(int)
+2 -2
View File
@@ -23,8 +23,8 @@ Use the `manage-qt-pyside6-lifetimes` skill for Qt ownership or lifecycle work.
- Native PySide and Nuitka can retain callbacks differently. Never pass a transient/repeated receiver's bound method to
`Signal.connect()` or `QTimer.singleShot()`, and do not hide that capture in a lambda/partial. Use
`connectWeakly(signal, receiver, 'methodName', sender=...)` when the sender is independent/longer-lived; use
`forwardSender=True` when the slot needs it. Direct bound methods are reserved for deliberately process-lifetime
receivers whose retention is intentional and documented.
`forwardSender=True` when the slot needs it, and use `singleShotWeakly()` for deferred named-method dispatch. Direct
bound methods are reserved for deliberately process-lifetime receivers whose retention is intentional and documented.
- `AppQAction.callback` is deliberately strong; its owner cannot outlive the receiver it captures. Parent or explicitly
dispose timers, models, delegates, replies, event filters, menus, actions, shortcuts, watchers, animations, and effects.
- Only the GUI thread mutates widgets. Slots do not sleep or perform unbounded host/network/process work. Create
+19 -10
View File
@@ -21,6 +21,7 @@ from __future__ import annotations
from Furious.Frozenlib import *
from Furious.Qt.QtNetwork import *
from Furious.Qt.Signals import connectWeakly
from PySide6 import QtCore
from PySide6.QtNetwork import *
@@ -87,22 +88,18 @@ class HttpGetManager(AppQNetworkAccessManager):
"""Handle ready read by network reply."""
self.hasDataCallback(networkReply, **kwargs)
@QtCore.Slot()
def _handleReadyRead(self):
@QtCore.Slot(object)
def _handleReadyRead(self, networkReply):
"""Dispatch ready-read data without a closure retaining the reply."""
networkReply = self.sender()
if isinstance(networkReply, QNetworkReply):
self.handleReadyReadByNetworkReply(
networkReply,
**self._replyContexts.get(networkReply, {}),
)
@QtCore.Slot()
def _handleFinished(self):
@QtCore.Slot(object)
def _handleFinished(self, networkReply):
"""Dispatch and release one completed network reply."""
networkReply = self.sender()
if not isinstance(networkReply, QNetworkReply):
return
@@ -171,7 +168,19 @@ class HttpGetManager(AppQNetworkAccessManager):
self._replyContexts[networkReply] = dict(kwargs)
networkReply.readyRead.connect(self._handleReadyRead)
networkReply.finished.connect(self._handleFinished)
connectWeakly(
networkReply.readyRead,
self,
'_handleReadyRead',
sender=networkReply,
forwardSender=True,
)
connectWeakly(
networkReply.finished,
self,
'_handleFinished',
sender=networkReply,
forwardSender=True,
)
return networkReply
+33 -6
View File
@@ -27,7 +27,7 @@ from typing import Any
import weakref
__all__ = ['connectWeakly']
__all__ = ['connectWeakly', 'singleShotWeakly']
def _ownsQObject(owner, object_) -> bool:
@@ -43,17 +43,14 @@ def _ownsQObject(owner, object_) -> bool:
return False
def connectWeakly(
signal,
def _weakMethodInvoker(
receiver: Any,
methodName: str,
*,
sender=None,
forwardSender: bool = False,
):
"""Connect without strongly owning a transient receiver or sender."""
# A plain dispatcher is intentional. Nuitka's PySide6 compatibility layer
# process-globally protects compiled bound methods passed directly to connect().
"""Return a plain callable that weakly dispatches to one named method."""
if not isinstance(methodName, str) or not methodName:
raise ValueError('method name must be a non-empty string')
@@ -88,6 +85,26 @@ def connectWeakly(
return method(currentSender, *args, **kwargs)
return invoke
def connectWeakly(
signal,
receiver: Any,
methodName: str,
*,
sender=None,
forwardSender: bool = False,
):
"""Connect without strongly owning a transient receiver or sender."""
# A plain dispatcher is intentional. Nuitka's PySide6 compatibility layer
# process-globally protects compiled bound methods passed directly to connect().
invoke = _weakMethodInvoker(
receiver,
methodName,
sender=sender,
forwardSender=forwardSender,
)
connection = signal.connect(invoke)
if (
@@ -107,3 +124,13 @@ def connectWeakly(
receiver.destroyed.connect(disconnect)
return connection
def singleShotWeakly(milliseconds: int, receiver: Any, methodName: str):
"""Schedule one named method without retaining its receiver."""
# QTimer.singleShot() is patched by the packaged runtime for compiled bound
# methods just like SignalInstance.connect(). Keep the scheduled callable
# plain and resolve the receiver only if it still exists when the timer fires.
invoke = _weakMethodInvoker(receiver, methodName)
QtCore.QTimer.singleShot(milliseconds, invoke)
+2 -1
View File
@@ -52,7 +52,7 @@ from .QtGui import (
bootstrapIconWithOpacity,
)
from .QtNetwork import AppQNetworkAccessManager
from .Signals import connectWeakly
from .Signals import connectWeakly, singleShotWeakly
from .QtWidgets import (
AppQComboBox,
AppQComboBoxSeparatorDelegate,
@@ -168,6 +168,7 @@ __all__ = [
'connectWeakly',
'gettext',
'moveToCenter',
'singleShotWeakly',
'showMBoxDirectRulesNotAllowed',
'showMBoxNewChangesNextTime',
'showMBoxUnrecognizedConfig',
+16 -11
View File
@@ -26,6 +26,7 @@ from Furious.Frozenlib import (
registerAppSettings,
)
from Furious.Qt.QtNetwork import AppQNetworkAccessManager
from Furious.Qt.Signals import connectWeakly
from Furious.Repository import Storage
from PySide6 import QtCore
@@ -51,6 +52,8 @@ __all__ = [
logger = logging.getLogger(__name__)
_MISSING_REQUEST = object()
PROXY_ENDPOINT_INFO_SETTING = 'ProxyEndpointInformationEnabled'
registerAppSettings(
@@ -142,19 +145,26 @@ class ProxyEndpointHttpClient(AppQNetworkAccessManager):
self._pendingRequests[reply] = context
reply.finished.connect(self._replyFinished)
connectWeakly(
reply.finished,
self,
'_replyFinished',
sender=reply,
forwardSender=True,
)
@QtCore.Slot()
def _replyFinished(self):
@QtCore.Slot(object)
def _replyFinished(self, reply):
"""Consume, publish, and release one completed reply."""
reply = self.sender()
if not isinstance(reply, QNetworkReply):
return
context = self._pendingRequests.pop(reply, None)
context = self._pendingRequests.pop(reply, _MISSING_REQUEST)
try:
if context is _MISSING_REQUEST:
return
if reply.error() == QNetworkReply.NetworkError.NoError:
data, error = bytes(reply.readAll()), ''
else:
@@ -171,11 +181,6 @@ class ProxyEndpointHttpClient(AppQNetworkAccessManager):
self._pendingRequests.clear()
for reply in pendingReplies:
try:
reply.finished.disconnect(self._replyFinished)
except (RuntimeError, TypeError):
pass
reply.abort()
reply.deleteLater()
+23 -12
View File
@@ -26,6 +26,7 @@ from Furious.Frozenlib import (
AppSettings,
)
from Furious.Qt.HttpGetManager import HttpGetManager
from Furious.Qt.Signals import connectWeakly
from Furious.Repository import Storage
from Furious.Service.SubscriptionImporter import (
SubscriptionImportService,
@@ -155,11 +156,9 @@ class SubscriptionManager(HttpGetManager):
and subscription.get('webURL') == kwargs.get('webURL')
)
@QtCore.Slot()
def _autoUpdateTimeout(self):
@QtCore.Slot(object)
def _autoUpdateTimeout(self, timer):
"""Run the subscription associated with the firing service-owned timer."""
timer = self.sender()
if not isinstance(timer, QtCore.QTimer):
return
@@ -199,7 +198,14 @@ class SubscriptionManager(HttpGetManager):
if timer is None:
timer = QtCore.QTimer(self)
timer.setProperty('subscriptionId', unique)
timer.timeout.connect(self._autoUpdateTimeout)
connectWeakly(
timer.timeout,
self,
'_autoUpdateTimeout',
sender=timer,
forwardSender=True,
)
self._autoUpdateTimers[unique] = timer
@@ -213,7 +219,7 @@ class SubscriptionManager(HttpGetManager):
logger.info(
f'stop auto update job for subscription '
f'({subscription.get("remark", "")}, {unique})'
f'({subscription.get("remark", "")}, {unique!r})'
)
return
@@ -228,13 +234,13 @@ class SubscriptionManager(HttpGetManager):
if previousInterval is None:
logger.info(
f'start auto update job for subscription '
f'({subscription.get("remark", "")}, {unique}). '
f'({subscription.get("remark", "")}, {unique!r}). '
f'Interval is {interval // (60 * 1000)} mins'
)
else:
logger.info(
f'reschedule auto update job for subscription '
f'({subscription.get("remark", "")}, {unique}). '
f'({subscription.get("remark", "")}, {unique!r}). '
f'Interval changed from {previousInterval // (60 * 1000)} '
f'to {interval // (60 * 1000)} mins'
)
@@ -273,10 +279,9 @@ class SubscriptionManager(HttpGetManager):
self._pruneRequestVersion(unique)
@QtCore.Slot()
def _releaseFinishedReply(self):
@QtCore.Slot(object)
def _releaseFinishedReply(self, reply):
"""Forget one exact subscription reply after its completion is dispatched."""
reply = self.sender()
unique = self._replySubscriptions.pop(reply, '')
self._activeReplies.pop(reply, None)
@@ -577,7 +582,13 @@ class SubscriptionManager(HttpGetManager):
self._activeReplies[reply] = reply
self._replySubscriptions[reply] = str(kwargs.get('unique', ''))
reply.finished.connect(self._releaseFinishedReply)
connectWeakly(
reply.finished,
self,
'_releaseFinishedReply',
sender=reply,
forwardSender=True,
)
def updateSubsByUnique(self, unique: str, **kwargs):
"""Update one enabled subscription group by stable ID."""
+28 -9
View File
@@ -30,7 +30,7 @@ from Furious.Plugins import (
profileFromAny,
)
from Furious.Qt import *
from Furious.Qt.Signals import connectWeakly
from Furious.Qt.Signals import connectWeakly, singleShotWeakly
from Furious.Qt import gettext as _
from Furious.Service import (
CORE_LOG_CATEGORY,
@@ -279,7 +279,13 @@ class TestDownloadSpeedWorker(HttpGetManager):
self.timeoutTimer = QtCore.QTimer(self)
self.timeoutTimer.setSingleShot(True)
self.timeoutTimer.timeout.connect(self.handleTimeout)
connectWeakly(
self.timeoutTimer.timeout,
self,
'handleTimeout',
sender=self.timeoutTimer,
)
def completionCallback(self, **kwargs):
"""Perform the required completion hook."""
@@ -544,7 +550,7 @@ class DownloadSpeedTestScheduler(QtCore.QObject):
self.drainScheduled = True
QtCore.QTimer.singleShot(0, self.drain)
singleShotWeakly(0, self, 'drain')
def drain(self):
"""Handle drain for the download speed test scheduler."""
@@ -619,7 +625,13 @@ class DownloadSpeedTestScheduler(QtCore.QObject):
self.activeJobs[id(worker)] = (worker, job, port)
worker.finished.connect(self.handleWorkerFinished)
connectWeakly(
worker.finished,
self,
'handleWorkerFinished',
sender=worker,
)
worker.start()
@QtCore.Slot(object)
@@ -678,7 +690,13 @@ class DeleteServersProgressDialog(AppQTransientDialog):
self.detailLabel = AppQLabel()
self.detailLabel.setWordWrap(True)
self.cancelButton = AppQPushButton(_('Cancel'))
self.cancelButton.clicked.connect(self.cancel)
connectWeakly(
self.cancelButton.clicked,
self,
'cancel',
sender=self.cancelButton,
)
statusLayout = QHBoxLayout()
statusLayout.addWidget(self.spinner)
@@ -697,7 +715,7 @@ class DeleteServersProgressDialog(AppQTransientDialog):
"""Open the delete servers progress dialog asynchronously."""
self.spinner.start()
QtCore.QTimer.singleShot(0, self.deleteNext)
singleShotWeakly(0, self, 'deleteNext')
return super().open()
@@ -705,7 +723,7 @@ class DeleteServersProgressDialog(AppQTransientDialog):
"""Reject the current delete servers progress dialog values."""
self.cancel()
def cancel(self):
def cancel(self, *_args):
"""Cancel the delete servers progress dialog operation."""
self.canceled = True
self.cancelButton.setEnabled(False)
@@ -750,7 +768,8 @@ class DeleteServersProgressDialog(AppQTransientDialog):
if deleteIndex < 0 or deleteIndex >= len(Storage.UserServers()):
self.updateStatus()
QtCore.QTimer.singleShot(0, self.deleteNext)
singleShotWeakly(0, self, 'deleteNext')
return
@@ -781,7 +800,7 @@ class DeleteServersProgressDialog(AppQTransientDialog):
self.deletedCount += 1
self.updateStatus()
QtCore.QTimer.singleShot(0, self.deleteNext)
singleShotWeakly(0, self, 'deleteNext')
def finishDeletion(self):
"""Handle finish deletion for the delete servers progress dialog."""
+10 -5
View File
@@ -367,16 +367,21 @@ class TextEditorWindow(AppQMainWindow):
def setIndent(self):
"""Set indent."""
indentSpinBox = IndentDialog(parent=self)
indentSpinBox.finished.connect(self._indentDialogFinished)
connectWeakly(
indentSpinBox.finished,
self,
'_indentDialogFinished',
sender=indentSpinBox,
forwardSender=True,
)
# Show the MessageBox asynchronously
indentSpinBox.open()
@QtCore.Slot(int)
def _indentDialogFinished(self, code):
@QtCore.Slot(object, int)
def _indentDialogFinished(self, indentSpinBox, code):
"""Apply the selected indentation without retaining the dialog."""
indentSpinBox = self.sender()
if not isinstance(indentSpinBox, IndentDialog):
return
+113
View File
@@ -34,6 +34,7 @@ from Furious.Backends.Xray.TrojanEditor import TrojanEditor
from Furious.Backends.Xray.TunSettingsDialog import XrayTunSettingsDialog
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.Qt import (
AppQAction,
@@ -44,11 +45,14 @@ from Furious.Qt import (
AppQSwitch,
AppQTransientDialog,
connectWeakly,
singleShotWeakly,
)
from Furious.Qt.QtWidgets import _AppMessageBoxMask
from Furious.Window.QRCodeWindow import QRCodeWindow, _QRCodePage
from Furious.Window.SubscriptionPage import _SubscriptionEditorDialog
from Furious.Window.IndentDialog import IndentDialog
from Furious.Window.TextEditorWindow import TextEditorWindow
from Furious.Widget.ServerTableView import DeleteServersProgressDialog
from PySide6 import QtCore
from PySide6.QtGui import QImage
@@ -68,6 +72,7 @@ from tests.support import (
import gc
import unittest
from unittest import mock
import weakref
@@ -117,6 +122,21 @@ class TransientReceiver(AppQTransientDialog):
self._calls.append(1)
class DelayedReceiver(QtCore.QObject):
"""Record weakly scheduled work while this receiver remains alive."""
def __init__(self, calls):
"""Retain only the caller-owned result list."""
super().__init__()
self._calls = calls
@QtCore.Slot()
def record(self):
"""Record one delivered timer callback."""
self._calls.append(1)
class QtLifetimeTest(unittest.TestCase):
"""Stress direct destruction evidence without relying on process RSS alone."""
@@ -309,6 +329,99 @@ class QtLifetimeTest(unittest.TestCase):
emitter.deleteLater()
def testWeakSingleShotDispatchDoesNotRetainDestroyedReceiver(self):
"""Run live work once and drop deferred work for a dead receiver."""
calls = []
liveReceiver = DelayedReceiver(calls)
singleShotWeakly(0, liveReceiver, 'record')
processQtEvents()
self.assertEqual(calls, [1])
deadReceiver = DelayedReceiver(calls)
deadReference = weakref.ref(deadReceiver)
singleShotWeakly(0, deadReceiver, 'record')
del deadReceiver
self.assertIsNone(deadReference())
processQtEvents()
self.assertEqual(calls, [1])
liveReceiver.deleteLater()
def testImportProgressWeakSchedulingCompletesAndDestroysEveryDialog(self):
"""Keep cooperative import callbacks outside packaged bound-method retention."""
iterations = 40
references = []
destroyed = []
with mock.patch(
'Furious.Actions.Import.Storage.UserServers',
return_value=[],
):
for _index in range(iterations):
dialog = ImportURIsProgressDialog(tuple())
dialog.destroyed.connect(lambda *_args: destroyed.append(True))
references.append(weakref.ref(dialog))
dialog.open()
processQtEvents()
del dialog
collectAtBoundary()
self.assertAllDestroyed(references, destroyed, iterations)
def testDeleteProgressWeakSchedulingCompletesAndDestroysEveryDialog(self):
"""Release empty cooperative delete dialogs after their scheduled work."""
iterations = 40
table = mock.Mock()
references = []
destroyed = []
for _index in range(iterations):
dialog = DeleteServersProgressDialog(table, tuple())
dialog.destroyed.connect(lambda *_args: destroyed.append(True))
references.append(weakref.ref(dialog))
dialog.open()
processQtEvents()
del dialog
collectAtBoundary()
self.assertAllDestroyed(references, destroyed, iterations)
self.assertEqual(table.sourceModel.refreshIndexes.call_count, iterations)
self.assertEqual(table.sourceModel.emitAllChanged.call_count, iterations)
def testTextEditorIndentCompletionReceivesExactTransientDialog(self):
"""Apply indentation through explicit weak sender forwarding."""
with isolatedSettings():
editor = TextEditorWindow()
editor.setPlainText('{"value": 1}', False)
editor.setIndent()
dialogs = editor.findChildren(IndentDialog)
self.assertEqual(len(dialogs), 1)
dialog = dialogs[0]
dialog.indentSpin.setValue(4)
dialog.accept()
processQtEvents()
self.assertIn('\n "value": 1\n', editor.jsonEditor.toPlainText())
editor.deleteLater()
def testHysteria2SwitchAnimationStopsWithTransientEditor(self):
"""Destroy owned switch animations even when a toggle just started."""
references = []
+41
View File
@@ -169,6 +169,47 @@ class SubscriptionManagerTest(TestCase):
self.assertFalse(hasattr(manager, 'table'))
manager.deleteLater()
def testSubscriptionDiagnosticsIncludeConfiguredURL(self):
"""Identify a request with its configured remark and URL."""
manager = self._manager()
profile = SimpleNamespace(itemRemark='profile')
manager.importer = SimpleNamespace(
importPayload=mock.Mock(
return_value=SimpleNamespace(
decoderId='decoder',
profiles=(profile,),
rejectedItems=0,
)
)
)
configuredURL = 'https://example.invalid/subscription?token=value'
with mock.patch('Furious.Service.SubscriptionManager.logger.info') as infoLog:
manager.successCallback(
_Reply(b'payload'),
unique='group-a',
remark='Group A',
webURL=configuredURL,
successArgs=[],
failureArgs=[],
)
with mock.patch('Furious.Service.SubscriptionManager.logger.error') as errorLog:
manager.failureCallback(
_Reply(error='offline'),
unique='group-a',
remark='Group A',
webURL=configuredURL,
failureArgs=[],
)
self.assertIn('Group A', infoLog.call_args.args[0])
self.assertIn('Group A', errorLog.call_args.args[0])
self.assertIn(configuredURL, infoLog.call_args.args[0])
self.assertIn(configuredURL, errorLog.call_args.args[0])
manager.deleteLater()
def testStaleRequestCompletionCannotMutateCurrentSubscription(self):
subscriptions = {
'group-a': {