diff --git a/Furious/Qt/HttpGetManager.py b/Furious/Qt/HttpGetManager.py index d7f5428..81cc53b 100644 --- a/Furious/Qt/HttpGetManager.py +++ b/Furious/Qt/HttpGetManager.py @@ -26,6 +26,8 @@ from Furious.Qt.Signals import connectWeakly from PySide6 import QtCore from PySide6.QtNetwork import * +from shiboken6 import isValid + from typing import Union import logging @@ -135,13 +137,16 @@ class HttpGetManager(AppQNetworkAccessManager): self.successCallback(networkReply, **kwargs) finally: try: - self.runCompletionCallback(**kwargs) + if isValid(self): + self.runCompletionCallback(**kwargs) finally: # QNetworkAccessManager owns replies by default and does not # remove completed children automatically. All response data # has been consumed by this point. The shared slots above use # weak sender forwarding, without a closure retaining this wrapper. - networkReply.deleteLater() + # User hooks can destroy the reply or its manager synchronously. + if isValid(networkReply): + networkReply.deleteLater() def configureHttpProxy(self, httpProxy: Union[str, None]) -> bool: """Configure HTTP proxy.""" diff --git a/Furious/Service/EndpointInfoService.py b/Furious/Service/EndpointInfoService.py index 84f8b48..dc74917 100644 --- a/Furious/Service/EndpointInfoService.py +++ b/Furious/Service/EndpointInfoService.py @@ -32,6 +32,8 @@ from Furious.Repository import Storage from PySide6 import QtCore from PySide6.QtNetwork import QNetworkReply, QNetworkRequest +from shiboken6 import isValid + from enum import Enum from dataclasses import dataclass, replace from ipaddress import IPv4Address, IPv6Address, ip_address @@ -172,7 +174,8 @@ class ProxyEndpointHttpClient(AppQNetworkAccessManager): self.completed.emit(context, data, error) finally: - reply.deleteLater() + if isValid(reply): + reply.deleteLater() def cancelAll(self): """Abort all connection-specific requests without retaining replies.""" @@ -181,8 +184,12 @@ class ProxyEndpointHttpClient(AppQNetworkAccessManager): self._pendingRequests.clear() for reply in pendingReplies: - reply.abort() - reply.deleteLater() + if isValid(reply): + reply.abort() + + # abort() can synchronously notify listeners that destroy this owner. + if isValid(reply): + reply.deleteLater() @dataclass(frozen=True) diff --git a/tests/test_service_runtime.py b/tests/test_service_runtime.py index d5c7272..dc6a66d 100644 --- a/tests/test_service_runtime.py +++ b/tests/test_service_runtime.py @@ -36,7 +36,7 @@ from PySide6 import QtCore from PySide6.QtNetwork import QNetworkReply from PySide6.QtWidgets import QWidget -from shiboken6 import isValid +from shiboken6 import isValid, delete as deleteQObject from tests.support import processQtEvents, application, collectAtBoundary, waitFor @@ -154,6 +154,67 @@ class HttpGetManagerLifetimeTest(unittest.TestCase): def setUpClass(cls): application() + def testCompletionMayDestroyReplyOrManager(self): + """Real finished delivery tolerates native deletion from user callbacks.""" + for managerType, contextAttribute in ( + (HttpGetManager, '_replyContexts'), + (ProxyEndpointHttpClient, '_pendingRequests'), + ): + for deleteManager in (False, True): + with self.subTest(manager=managerType.__name__, owner=deleteManager): + for _ in range(20): + manager = managerType() + reply = _ManagedReply(manager) + destroyed = [] + reply.destroyed.connect(lambda *_a: destroyed.append(True)) + completion = [] + + def destroyFromCallback(*_args, **_kwargs): + deleteQObject(manager if deleteManager else reply) + + try: + with patch.object(manager, 'get', lambda _request: reply): + if isinstance(manager, HttpGetManager): + manager.webGET( + 'https://invalid.test', logActionMessage=False + ) + manager.successCallback = destroyFromCallback + manager.completionCallback = ( + lambda **_k: completion.append(True) + ) + else: + manager.request('https://invalid.test', 'fixture') + manager.completed.connect(destroyFromCallback) + + with patch('sys.excepthook') as exceptionHook: + reply.finished.emit() + processQtEvents() + exceptionHook.assert_not_called() + self.assertEqual(destroyed, [True]) + self.assertFalse(isValid(reply)) + self.assertFalse(getattr(manager, contextAttribute)) + if isinstance(manager, HttpGetManager): + self.assertEqual( + completion, [] if deleteManager else [True] + ) + finally: + if isValid(manager): + deleteQObject(manager) + processQtEvents() + + def testEndpointCancellationToleratesOwnerDestructionDuringAbort(self): + """An abort listener may delete the manager and its other pending replies.""" + manager = ProxyEndpointHttpClient() + replies = [_ManagedReply(manager), _ManagedReply(manager)] + for index, reply in enumerate(replies): + with patch.object(manager, 'get', lambda _request: reply): + manager.request('https://invalid.test', index) + replies[0].abort = lambda: deleteQObject(manager) + manager.cancelAll() + self.assertFalse(isValid(manager)) + self.assertTrue(all(not isValid(reply) for reply in replies)) + self.assertEqual(manager._pendingRequests, {}) + def testRequestHasFiniteTimeoutAndTerminalPathDropsContext(self): manager = _CapturingHttpGetManager()