diff --git a/Furious/Backends/Xray/AssetDownloadManager.py b/Furious/Backends/Xray/AssetDownloadManager.py index 4d1800d..0188249 100644 --- a/Furious/Backends/Xray/AssetDownloadManager.py +++ b/Furious/Backends/Xray/AssetDownloadManager.py @@ -24,6 +24,8 @@ from Furious.Qt import * from PySide6 import QtCore +from shiboken6 import isValid + from typing import AnyStr, Union, Callable import os @@ -31,31 +33,76 @@ import re import logging import hashlib import functools +import weakref __all__ = ['XrayAssetDownloadManager'] logger = logging.getLogger(__name__) -class SHA256Worker(QtCore.QObject, QtCore.QRunnable): - """Run SHA-256 work in the background.""" +class _SHA256ResultEvent(QtCore.QEvent): + """Deliver one immutable hash result to the manager's Qt thread.""" - finished = QtCore.Signal(str) + Type = QtCore.QEvent.Type(QtCore.QEvent.registerEventType()) - def __init__(self, string=b''): - # Explictly called __init__ - """Initialize the SHA256Worker.""" - QtCore.QObject.__init__(self) - QtCore.QRunnable.__init__(self) + def __init__(self, token, digest): + super().__init__(self.Type) - self.string = string + self.token = token + self.digest = digest + + +class SHA256Worker(QtCore.QRunnable): + """Hash copied bytes without owning Qt objects or download callbacks.""" + + def __init__(self, receiver, token, data): + super().__init__() + + self._receiver = weakref.ref(receiver) + self._token = token + self._data = data def run(self): - """Run the SHA-256 worker task.""" - self.finished.emit(hashlib.sha256(self.string).hexdigest()) + """Post a result only while its manager still has a native Qt object.""" + digest = hashlib.sha256(self._data).hexdigest() + + receiver = self._receiver() + + if receiver is not None and isValid(receiver): + try: + QtCore.QCoreApplication.postEvent( + receiver, _SHA256ResultEvent(self._token, digest) + ) + except RuntimeError: + # Native destruction can race the validity check on this thread. + pass -class XrayAssetSHA256DownloadManager(HttpGetManager): +class _AssetDownloadClient(HttpGetManager): + """Own asset requests until the plugin closes this client.""" + + def __init__(self, parent=None, **kwargs): + super().__init__(parent, **kwargs) + self._closed = False + + def shutdown(self): + """Reject late success and abort this client's exact pending replies.""" + if self._closed: + return + + self._closed = True + + for reply in tuple(self._replyContexts): + if isValid(reply) and not reply.isFinished(): + reply.abort() + + self._replyContexts.clear() + + if isValid(self): + self.deleteLater() + + +class XrayAssetSHA256DownloadManager(_AssetDownloadClient): """Coordinate Xray asset SHA-256 download operations.""" def __init__(self, parent=None, **kwargs): @@ -64,6 +111,17 @@ class XrayAssetSHA256DownloadManager(HttpGetManager): super().__init__(parent, actionMessage=actionMessage) + self._hashJobs = {} + + # A surviving Python wrapper must not retain callbacks after Qt teardown. + self.destroyed.connect(self._hashJobs.clear) + + def shutdown(self): + """Discard hash callback contexts before aborting metadata requests.""" + self._hashJobs.clear() + + super().shutdown() + @staticmethod def fileContent(filepath, mode='rb') -> AnyStr: """Return the file content value used by the Xray asset SHA-256 download manager.""" @@ -92,6 +150,9 @@ class XrayAssetSHA256DownloadManager(HttpGetManager): def successCallback(self, networkReply, **kwargs): """Handle a successful network operation.""" + if self._closed: + return + filepath = kwargs.pop('filepath', '') downloadCallback = kwargs.pop('downloadCallback', None) @@ -107,29 +168,51 @@ class XrayAssetSHA256DownloadManager(HttpGetManager): return - def handleFinished(_digest, _value=''): - """Handle finished.""" - logger.debug( - f'computed digest is \'{_digest}\' while repo digest is \'{_value}\'' - ) + token = object() + worker = SHA256Worker(self, token, self.fileContent(filepath)) - if _digest != _value: - logger.info(f'digest not equal for {basename}. Start downloading asset') + self._hashJobs[token] = (basename, value, downloadCallback) - if callable(downloadCallback): - downloadCallback(_value) - else: - logger.info(f'digest equal for {basename}. Nothing to do') + try: + AppThreadPool().start(worker) + except Exception: + # Any non-exit exceptions - worker = SHA256Worker(self.fileContent(filepath)) + self._hashJobs.pop(token, None) - worker.setAutoDelete(True) - worker.finished.connect(functools.partial(handleFinished, _value=value)) + raise - AppThreadPool().start(worker) + def event(self, event): + """Consume hash jobs exactly once, in the downloader's owning thread.""" + if event.type() != _SHA256ResultEvent.Type: + return super().event(event) + + context = self._hashJobs.pop(event.token, None) + + if context is None: + return True + + basename, expectedDigest, downloadCallback = context + + logger.debug( + f'computed digest is {event.digest!r} while repo digest is {expectedDigest!r}' + ) + + if event.digest != expectedDigest: + logger.info(f'digest not equal for {basename}. Start downloading asset') + + if callable(downloadCallback): + downloadCallback(expectedDigest) + else: + logger.info(f'digest equal for {basename}. Nothing to do') + + return True def download(self, url, filepath, downloadCallback: Callable[[str], None]): """Download the Xray asset SHA-256 download manager.""" + if self._closed: + return + self.webGET( url, filepath=str(filepath), @@ -137,7 +220,7 @@ class XrayAssetSHA256DownloadManager(HttpGetManager): ) -class XrayAssetAssetsDownloadManager(HttpGetManager): +class XrayAssetAssetsDownloadManager(_AssetDownloadClient): """Coordinate Xray asset assets download operations.""" def __init__(self, parent=None, **kwargs): @@ -148,6 +231,9 @@ class XrayAssetAssetsDownloadManager(HttpGetManager): def successCallback(self, networkReply, **kwargs): """Handle a successful network operation.""" + if self._closed: + return + filepath = kwargs.pop('filepath', '') expectedDigest = str(kwargs.pop('expectedDigest', '')).lower() @@ -191,6 +277,9 @@ class XrayAssetAssetsDownloadManager(HttpGetManager): def download(self, url, filepath, expectedDigest: str): """Download the Xray asset assets download manager.""" + if self._closed: + return + self.webGET( url, filepath=str(filepath), @@ -215,6 +304,11 @@ class XrayAssetPairDownloadHelper: actionMessage=assetsActionMessage ) + def shutdown(self): + """Close both stages of this asset update.""" + self.sha256Downloader.shutdown() + self.assetsDownloader.shutdown() + def configureHttpProxy(self, httpProxy: Union[str, None]) -> bool: """Configure HTTP proxy.""" return all( @@ -251,6 +345,11 @@ class XrayAssetDownloadManager: assetsActionMessage='download geoip assets', ) + def shutdown(self): + """Release every client acquired by the plugin's asset updater.""" + self.downloadHelperGeosite.shutdown() + self.downloadHelperGeoip.shutdown() + def configureHttpProxy(self, httpProxy: Union[str, None]) -> bool: """Configure HTTP proxy.""" return all( diff --git a/Furious/Backends/Xray/Plugin.py b/Furious/Backends/Xray/Plugin.py index ab7e683..02bff7f 100644 --- a/Furious/Backends/Xray/Plugin.py +++ b/Furious/Backends/Xray/Plugin.py @@ -374,6 +374,15 @@ class XrayCoreRuntimeFactory(CoreRuntimeFactory): """Return the timestamp format emitted by Xray-core.""" return (r'\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2}.\d{6}',) + def shutdown(self): + """Release the lazily acquired asset updater before Qt teardown.""" + manager = getattr(self, '_assetDownloadManager', None) + + if manager is not None: + manager.shutdown() + + self._assetDownloadManager = None + def afterConnected(self, httpProxy=None): """Update Xray geo assets after connecting when enabled.""" if not SystemRuntime.isAssetsFolderWritable(): @@ -428,10 +437,15 @@ class XrayPlugin(FuriousPlugin): def __init__(self): """Create an isolated Xray runtime factory for this plugin.""" + self._runtimeFactory = XrayCoreRuntimeFactory() self.capabilities = ( *XRAY_PROTOCOL_HANDLERS, *XRAY_PROTOCOL_EDITORS, - XrayCoreRuntimeFactory(), + self._runtimeFactory, XrayStatsProvider(), XrayActionProvider(), ) + + def shutdown(self): + """Close the runtime factory's reusable asset download service.""" + self._runtimeFactory.shutdown() diff --git a/tests/README.md b/tests/README.md index 08b2f4d..fc93fb8 100644 --- a/tests/README.md +++ b/tests/README.md @@ -200,3 +200,21 @@ started for the smoke test. at diagnostic batch boundaries, never once per UI operation. - A lifetime failure must be investigated as an ownership defect; increasing thresholds or forcing production garbage collection is not an acceptable fix. + +### Focused lifetime audit regressions + +`test_qt_lifetime.py` checks that independent signal endpoints do not accumulate +cleanup hooks when senders die first and releases message-box masks on native deletion. +`test_theme_transition.py` covers both target-window and coordinator destruction during a fade. `test_connection_startup_async.py` covers DNS +cancellation/timeout with already-deleted recursive replies. `test_service_runtime.py` +rejects deleted plugin-page wrappers. `test_xray_asset_download.py` exercises real +pool-thread delivery, early manager destruction, callback release, and plugin shutdown +of pending replies and hashes. + +Run `python -m tests.fixtures.editor_lifetime_probe --iterations 100 --pattern representative --close-method close` +natively and compile that fixture with Nuitka's PySide6 plugin for a separate standalone +check. It checks both independent signal endpoint destruction orders as well as seven +transient editor families. Repeat with `--close-method accept` and `--close-method reject`. +A null protected-list count means Nuitka does not expose that diagnostic; inspect its +installed package configuration and require zero live wrappers and registry entries +instead. diff --git a/tests/test_xray_asset_download.py b/tests/test_xray_asset_download.py index 8402202..58dbc78 100644 --- a/tests/test_xray_asset_download.py +++ b/tests/test_xray_asset_download.py @@ -19,10 +19,13 @@ from Furious.Backends.Xray.AssetDownloadManager import ( XrayAssetAssetsDownloadManager, + XrayAssetDownloadManager, XrayAssetSHA256DownloadManager, ) -from PySide6 import QtCore +from PySide6 import QtCore, QtNetwork + +from shiboken6 import isValid from unittest import TestCase, mock @@ -30,8 +33,10 @@ import os import hashlib import unittest import tempfile +import threading +import weakref -from tests.support import application, processQtEvents +from tests.support import application, processQtEvents, waitFor class _Reply: @@ -44,6 +49,21 @@ class _Reply: return self._data +class _PendingReply(QtNetwork.QNetworkReply): + """Exercise real reply signals without opening a network connection.""" + + def __init__(self, parent): + super().__init__(parent) + self.abortCount = 0 + + def abort(self): + self.abortCount += 1 + self.setError(self.NetworkError.OperationCanceledError, 'cancelled by test') + self.setFinished(True) + + self.finished.emit() + + class XrayAssetDownloadTest(TestCase): """Verify checksum metadata and downloaded bytes before replacement.""" @@ -54,6 +74,179 @@ class XrayAssetDownloadTest(TestCase): def tearDown(self): processQtEvents() + def testPluginShutdownReleasesAssetClientsAndPendingHashes(self): + """Plugin shutdown must reach the clients acquired after connection.""" + from Furious.Backends.Xray.Plugin import XrayPlugin, XrayCoreRuntimeFactory + + plugin = XrayPlugin() + factory = next( + item + for item in plugin.capabilities + if isinstance(item, XrayCoreRuntimeFactory) + ) + manager = XrayAssetDownloadManager() + factory._assetDownloadManager = manager + + clients = [ + client + for helper in (manager.downloadHelperGeosite, manager.downloadHelperGeoip) + for client in (helper.sha256Downloader, helper.assetsDownloader) + ] + download = mock.Mock() + pool = mock.Mock() + + with mock.patch( + 'Furious.Backends.Xray.AssetDownloadManager.AppThreadPool', + return_value=pool, + ): + clients[0].successCallback( + _Reply(hashlib.sha256(b'new asset').hexdigest().encode()), + filepath='missing-asset.dat', + downloadCallback=download, + ) + + replies = [] + + for client in clients: + reply = _PendingReply(client) + replies.append(reply) + + with mock.patch.object(client, 'get', return_value=reply): + client.webGET('https://example.invalid/asset', logActionMessage=False) + + worker = pool.start.call_args.args[0] + + try: + plugin.shutdown() + plugin.shutdown() + + worker.run() + processQtEvents() + + download.assert_not_called() + self.assertTrue(all(not isValid(client) for client in clients)) + self.assertEqual([reply.abortCount for reply in replies], [1] * 4) + self.assertTrue(all(not isValid(reply) for reply in replies)) + finally: + for client in clients: + if isValid(client): + client.deleteLater() + + processQtEvents() + + def testPoolHashResultsReturnToOwnerThreadAndReleaseJobs(self): + """Real pool execution crosses back to Qt before invoking callbacks.""" + manager = XrayAssetSHA256DownloadManager() + pool = QtCore.QThreadPool() + deliveries = [] + expected = hashlib.sha256(b'new asset').hexdigest() + + try: + with mock.patch( + 'Furious.Backends.Xray.AssetDownloadManager.AppThreadPool', + return_value=pool, + ): + for _ in range(30): + manager.successCallback( + _Reply(expected.encode()), + filepath='missing-asset.dat', + downloadCallback=lambda digest: deliveries.append( + (digest, QtCore.QThread.currentThread()) + ), + ) + + self.assertTrue(waitFor(lambda: len(deliveries) == 30)) + self.assertEqual(deliveries, [(expected, manager.thread())] * 30) + self.assertEqual(manager._hashJobs, {}) + finally: + self.assertTrue(pool.waitForDone(3000)) + manager.deleteLater() + pool.deleteLater() + processQtEvents() + + def testRunningHashDoesNotRetainCallbackAfterOwnerDestruction(self): + """A blocked real worker owns bytes, not a destroyed owner's callback.""" + manager = XrayAssetSHA256DownloadManager() + pool = QtCore.QThreadPool() + entered = threading.Event() + release = threading.Event() + digestFunction = hashlib.sha256 + download = mock.Mock() + callbackReference = weakref.ref(download) + expected = digestFunction(b'new asset').hexdigest() + + def compute(data): + entered.set() + + if not release.wait(3): + raise RuntimeError('test hash was not released') + + return digestFunction(data) + + try: + with ( + mock.patch( + 'Furious.Backends.Xray.AssetDownloadManager.AppThreadPool', + return_value=pool, + ), + mock.patch( + 'Furious.Backends.Xray.AssetDownloadManager.hashlib.sha256', + side_effect=compute, + ), + ): + manager.successCallback( + _Reply(expected.encode()), + filepath='missing-asset.dat', + downloadCallback=download, + ) + del download + + self.assertTrue(entered.wait(3)) + + manager.deleteLater() + processQtEvents() + + self.assertIsNone(callbackReference()) + + release.set() + self.assertTrue(pool.waitForDone(3000)) + processQtEvents() + finally: + release.set() + self.assertTrue(pool.waitForDone(3000)) + + if isValid(manager): + manager.deleteLater() + pool.deleteLater() + processQtEvents() + + def testHashCompletionCannotOutliveDownloadManager(self): + """A queued hash must not start downloads after its Qt owner dies.""" + manager = XrayAssetSHA256DownloadManager() + download = mock.Mock() + pool = mock.Mock() + + with mock.patch( + 'Furious.Backends.Xray.AssetDownloadManager.AppThreadPool', + return_value=pool, + ): + manager.successCallback( + _Reply(hashlib.sha256(b'new asset').hexdigest().encode()), + filepath='missing-asset.dat', + downloadCallback=download, + ) + + worker = pool.start.call_args.args[0] + + manager.deleteLater() + processQtEvents() + + worker.run() + processQtEvents() + manager.shutdown() + + download.assert_not_called() + def testMalformedChecksumDoesNotStartHashOrAssetDownload(self): manager = XrayAssetSHA256DownloadManager() download = mock.Mock() @@ -99,6 +292,8 @@ class XrayAssetDownloadTest(TestCase): downloadCallback=download, ) + processQtEvents() + download.assert_called_once_with(expectedDigest) manager.deleteLater()