Handle reentrant HTTP cleanup

Signed-off-by: Loren Eteval <loren.eteval@proton.me>

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."""

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)

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()