Fix Qt object lifetime cleanup

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

.agents/skills/manage-qt-pyside6-lifetimes/references/qt-pyside6-object-lifetime-guidelines.md

@@ -273,10 +273,12 @@ connectWeakly(
   `QObject.sender()`;
 - `_ownsQObject(receiver, sender)` walks from the sender through its `parent()` chain;
 - when that walk shows that the sender is not the receiver or its descendant,
-  `sender=` makes receiver destruction disconnect the otherwise dormant dispatcher;
-- that cleanup captures only the opaque connection returned by `signal.connect()` and
-  calls `QtCore.QObject.disconnect(connection)`; it deliberately does not capture the
-  sender's `SignalInstance`, which may wrap an already-deleted sender during teardown;
+  `sender=` makes either endpoint's destruction release dispatch and both cleanup hooks;
+- that cleanup captures only opaque connection handles and calls
+  `QtCore.QObject.disconnect(connection)`; it deliberately does not capture either
+  QObject or the sender's `SignalInstance`, which may wrap an already-deleted sender;
+- releasing both cleanup hooks prevents short-lived senders from accumulating callbacks
+  on a surviving receiver; verify both destruction orders over repeated cycles;
 - the method name is static and must remain valid for the receiver's lifetime.
 
 Pass the sender whenever it is outside the receiver's QObject subtree. Omitting it can
@@ -338,8 +340,10 @@ UI objects may observe controller state. Controllers should not become accidenta
 Long-running resources belong to the service that schedules them, not to a transient
 page callback. Furious's current profile-test and subscription services parent their
 manager-side QObjects/pools to durable owners, cross worker results back to the owning
-Qt thread, reject stale generations/identities, and provide bounded idempotent
-shutdown. Preserve that ownership shape when moving work off the GUI thread.
+Qt thread, and reject stale generations/identities. Their shutdown is idempotent, but
+not uniformly bounded: subscription preparation drains cooperatively and waits
+synchronously after its diagnostic timeout. Preserve ownership until workers finish;
+do not confuse a diagnostic timeout with a termination deadline.
 
 ## 12. Plugin and Registry Design
 

Furious/Qt/QtWidgets.py

@@ -1856,7 +1856,11 @@ class AppQMessageBox(AppQTransientDialog):
             return
 
         self._removeWindowMask()
+
         self._windowMask = _AppMessageBoxMask(owner)
+        # Native deletion can bypass done(); the window must not keep the mask.
+        self.destroyed.connect(self._windowMask.deleteLater)
+
         self._windowMask.show()
         self._windowMask.raise_()
 

Furious/Qt/Signals.py

@@ -146,15 +146,19 @@ def connectWeakly(
         and not _ownsQObject(receiver, sender)
     ):
 
-        def disconnect(*_args):
-            """Remove weak dispatch from an independently owned sender."""
-            # Retain only Qt's opaque connection handle. Capturing ``signal``
-            # here keeps the sender's SignalInstance wrapper alive until the
-            # receiver dies; when the native sender died first, PySide6 could
-            # then access that stale wrapper during application teardown.
-            QtCore.QObject.disconnect(connection)
+        connections = [connection]
 
-        receiver.destroyed.connect(disconnect)
+        def disconnect(*_args):
+            """Release dispatch and both cleanup hooks when either endpoint dies."""
+            # Keep only opaque handles, never SignalInstance wrappers or QObjects.
+            # Removing both hooks also bounds retention when senders die first.
+            for ownedConnection in connections:
+                QtCore.QObject.disconnect(ownedConnection)
+
+            connections.clear()
+
+        connections.append(receiver.destroyed.connect(disconnect))
+        connections.append(sender.destroyed.connect(disconnect))
 
     return connection
 

Furious/Qt/ThemeTransition.py

@@ -180,6 +180,11 @@ class ThemeTransition(QtCore.QObject):
                 b'opacityValue',
                 self,
             )
+
+            # The snapshot belongs to the window, but must also die if its
+            # animation owner is destroyed early. This is a native Qt slot.
+            animation.destroyed.connect(overlay.deleteLater)
+
             animation.setDuration(self._duration)
             animation.setStartValue(1.0)
             animation.setEndValue(0.0)
@@ -198,6 +203,13 @@ class ThemeTransition(QtCore.QObject):
                 forwardSender=True,
             )
 
+            connectWeakly(
+                overlay.destroyed,
+                self,
+                '_releaseDestroyedOverlays',
+                sender=overlay,
+            )
+
             overlay.show()
             overlay.raise_()
 
@@ -209,6 +221,12 @@ class ThemeTransition(QtCore.QObject):
         for animation in tuple(self._animations):
             animation.start()
 
+    def _releaseDestroyedOverlays(self):
+        """Native target destruction stops animations without emitting finished."""
+        for animation, (_window, overlay) in tuple(self._animations.items()):
+            if not isValid(overlay):
+                self._releaseAnimation(animation)
+
     def _releaseAnimation(self, animation, *, notify=True):
         """Release one animation and its transient overlay exactly once."""
         transition = self._animations.pop(animation, None)

Furious/Service/DnsResolver.py

@@ -27,6 +27,8 @@ from Furious.Qt.Signals import connectWeakly
 from PySide6 import QtCore
 from PySide6.QtNetwork import *
 
+from shiboken6 import isValid
+
 from typing import Tuple
 
 import logging
@@ -104,6 +106,7 @@ class DnsResolutionOperation(QtCore.QObject):
         for networkReply in self._resultMap['reference']:
             if (
                 isinstance(networkReply, QNetworkReply)
+                and isValid(networkReply)
                 and not networkReply.isFinished()
             ):
                 networkReply.abort()
@@ -372,6 +375,7 @@ class DnsResolver(HttpGetManager):
             for networkReply in resultMap['reference']:
                 if (
                     isinstance(networkReply, QNetworkReply)
+                    and isValid(networkReply)
                     and not networkReply.isFinished()
                 ):
                     networkReply.abort()
@@ -379,7 +383,7 @@ class DnsResolver(HttpGetManager):
     def dispose(self):
         """Abort pending replies and schedule this resolver for destruction."""
         for networkReply in tuple(self._replyContexts):
-            if not networkReply.isFinished():
+            if isValid(networkReply) and not networkReply.isFinished():
                 networkReply.abort()
 
         self._replyContexts.clear()

Furious/Service/PluginUIManager.py

@@ -29,6 +29,8 @@ from Furious.Plugins import (
 from PySide6 import QtCore
 from PySide6.QtWidgets import QWidget
 
+from shiboken6 import isValid
+
 import logging
 
 __all__ = ['PluginNavigationManager', 'isCoreActive']
@@ -110,10 +112,10 @@ class PluginNavigationManager:
 
                 continue
 
-            if not isinstance(page, QWidget):
-                logger.error(f'plugin page {pageId!r} did not create a QWidget')
+            if not isinstance(page, QWidget) or not isValid(page):
+                logger.error(f'plugin page {pageId!r} did not create a valid QWidget')
 
-                if isinstance(page, QtCore.QObject):
+                if isinstance(page, QtCore.QObject) and isValid(page):
                     page.deleteLater()
 
                 continue
@@ -131,7 +133,8 @@ class PluginNavigationManager:
 
                 logger.error(f'failed to register plugin page {pageId!r}: {ex}')
 
-                page.deleteLater()
+                if isValid(page):
+                    page.deleteLater()
 
                 continue
 

Furious/Widget/ServerTableView.py

@@ -753,6 +753,7 @@ class ServerTableView(
         self.proxyModel = UserServersSortFilterProxyModel(parent=self)
         self.proxyModel.setSourceModel(self.sourceModel)
         self.setModel(self.proxyModel)
+
         self._sortSelectionSnapshot = None
         self.proxyModel.sortAboutToStart.connect(self._captureSortSelection)
         self.proxyModel.sortCompleted.connect(self._restoreSortSelection)
@@ -878,6 +879,7 @@ class ServerTableView(
         self._subscriptionActions = []
 
         self.importActions = tuple(importActionsFactory())
+
         self.testActions = (
             AppQAction(
                 _('Test Ping Latency'),

tests/fixtures/editor_lifetime_probe.py

@@ -21,11 +21,12 @@ from __future__ import annotations
 
 from Furious.Backends import OFFICIAL_PLUGIN_TYPES
 from Furious.Plugins import blankProfile, initializePluginRegistry
-from Furious.Qt import AppQDialog, connectWeakly
+from Furious.Qt import AppQDialog, AppQMessageBox, ThemeTransition, connectWeakly
 from Furious.Widget.ServerTableView import ServerTableView
 
 import PySide6
 
+from PySide6 import QtCore
 from PySide6.QtWidgets import QWidget
 
 from shiboken6 import isValid
@@ -221,6 +222,116 @@ def runProbe(
             registry.shutdown()
 
 
+class _SignalEndpoint(QtCore.QObject):
+    """Exercise compiled methods with independently destroyed Qt endpoints."""
+
+    emitted = QtCore.Signal()
+
+    def __init__(self):
+        super().__init__()
+        self.calls = 0
+
+    def record(self):
+        self.calls += 1
+
+
+def runInfrastructureProbe(iterations=100):
+    """Check signal, mask, and animation ownership under real Qt destruction."""
+    application()
+    result = {}
+
+    for senderFirst in (True, False):
+        survivor = _SignalEndpoint()
+        counts = []
+
+        for _ in range(iterations):
+            transient = _SignalEndpoint()
+            sender, receiver = (
+                (transient, survivor) if senderFirst else (survivor, transient)
+            )
+
+            connectWeakly(sender.emitted, receiver, 'record', sender=sender)
+            sender.emitted.emit()
+
+            assert receiver.calls > 0
+
+            transient.deleteLater()
+            processQtEvents()
+
+            assert not isValid(transient)
+
+            survivor.emitted.emit()
+            counts.append(survivor.receivers(QtCore.SIGNAL('destroyed(QObject*)')))
+
+        assert counts == [counts[0]] * iterations, counts
+        result['senderFirst' if senderFirst else 'receiverFirst'] = iterations
+
+        survivor.deleteLater()
+        processQtEvents()
+
+    for windowFirst in (True, False):
+        for _ in range(iterations):
+            window = QWidget()
+            window.show()
+            processQtEvents()
+
+            transition = ThemeTransition(
+                duration=100000,
+                windowProvider=lambda: (window,),
+                animationsEnabled=lambda: True,
+            )
+
+            transition.apply(lambda: None)
+            animation = next(iter(transition._animations))
+            overlay = window.findChild(QWidget, transition.OverlayObjectName)
+
+            if windowFirst:
+                window.deleteLater()
+                processQtEvents()
+
+                assert not transition._animations
+                assert not transition._animationsByWindow
+
+                transition.deleteLater()
+            else:
+                transition.deleteLater()
+                processQtEvents()
+                window.deleteLater()
+
+            processQtEvents()
+
+            assert not isValid(animation)
+            assert not isValid(overlay)
+
+        result['themeWindowFirst' if windowFirst else 'themeCoordinatorFirst'] = (
+            iterations
+        )
+
+    owner = QWidget()
+    owner.show()
+    processQtEvents()
+
+    try:
+        for _ in range(iterations):
+            dialog = AppQMessageBox(parent=owner, text='Lifetime probe')
+            dialog.open()
+            mask = dialog._windowMask
+
+            dialog.deleteLater()
+            processQtEvents()
+
+            assert not isValid(dialog)
+            assert not isValid(mask)
+
+        assert not AppQDialog._openDialogs
+        result['messageBoxDeleted'] = iterations
+    finally:
+        owner.deleteLater()
+        processQtEvents()
+
+    return result
+
+
 def main():
     """Run the probe as a standalone source or Nuitka executable."""
     parser = argparse.ArgumentParser()
@@ -231,6 +342,8 @@ def main():
     parser.add_argument('--close-method', choices=CLOSE_METHODS, default='reject')
     arguments = parser.parse_args()
 
+    print(json.dumps(runInfrastructureProbe(arguments.iterations), sort_keys=True))
+
     print(
         json.dumps(
             runProbe(

tests/test_connection_startup_async.py

@@ -26,7 +26,7 @@ from Furious.Service.ConnectionManager import (
     ConnectionManager,
     ConnectionStartStage,
 )
-from Furious.Service.DnsResolver import DnsResolutionOperation
+from Furious.Service.DnsResolver import DnsResolutionOperation, DnsResolver
 
 from PySide6 import QtCore, QtNetwork
 
@@ -438,6 +438,48 @@ class ConnectionStartupAsyncTest(TestCase):
         self.assertEqual(runtime.startOptions, [{}])
         self.assertEqual(manager.runtimes, [runtime])
 
+    def testDnsCancellationAndTimeoutIgnoreAlreadyDestroyedReplies(self):
+        """Earlier recursive replies may be deleted before later requests stop."""
+
+        class PendingReply(QtNetwork.QNetworkReply):
+            def __init__(self):
+                super().__init__()
+                self.abortCount = 0
+
+            def abort(self):
+                self.abortCount += 1
+                self.setFinished(True)
+
+        for synchronous in (False, True):
+            with self.subTest(synchronous=synchronous):
+                resolver = mock.Mock()
+                resolver._newResultMap = DnsResolver._newResultMap
+                operation = DnsResolutionOperation(resolver, 'example.test')
+
+                completed = PendingReply()
+                pending = PendingReply()
+                operation._resultMap['reference'] = [completed, pending]
+                operation._resultMap['depth'] = 1
+
+                completed.deleteLater()
+                processQtEvents()
+
+                try:
+                    if synchronous:
+                        with self.assertLogs(
+                            'Furious.Service.DnsResolver', level='ERROR'
+                        ):
+                            DnsResolver.wait(operation._resultMap, timeout=0)
+                    else:
+                        operation.cancel()
+                        operation.cancel()
+
+                    self.assertEqual(pending.abortCount, 1)
+                finally:
+                    pending.deleteLater()
+                    operation.deleteLater()
+                    processQtEvents()
+
     def testDnsResolutionOperationCompletesAndCancelsWithoutNestedWait(self):
         """Observe recursive DNS state through timers and suppress stale cancel."""
         resolver = _ResolverFixture()
@@ -469,6 +511,7 @@ class ConnectionStartupAsyncTest(TestCase):
         processQtEvents(5)
 
         self.assertEqual(staleResults, [])
+
         operation.deleteLater()
         cancelled.deleteLater()
         resolver.deleteLater()

tests/test_qt_lifetime.py

@@ -140,6 +140,23 @@ class DelayedReceiver(QtCore.QObject):
 class QtLifetimeTest(unittest.TestCase):
     """Stress direct destruction evidence without relying on process RSS alone."""
 
+    def testIndependentSenderDestructionReleasesReceiverCleanupHooks(self):
+        """A surviving receiver must not accumulate hooks for dead senders."""
+        receiver = DelayedReceiver([])
+        self.addCleanup(receiver.deleteLater)
+        counts = []
+
+        for _ in range(40):
+            sender = LongLivedEmitter()
+            connectWeakly(sender.emitted, receiver, 'record', sender=sender)
+
+            sender.deleteLater()
+            processQtEvents()
+
+            counts.append(receiver.receivers(QtCore.SIGNAL('destroyed(QObject*)')))
+
+        self.assertEqual(counts, [counts[0]] * len(counts))
+
     @classmethod
     def setUpClass(cls):
         """Create the one QApplication used by the entire test process."""
@@ -290,6 +307,30 @@ class QtLifetimeTest(unittest.TestCase):
         self.assertTrue(waitFor(lambda: reference() is None))
         self.assertNotIn(key, AppQMainWindow._openWindows)
 
+    def testNativeMessageBoxDestructionReleasesItsWindowMask(self):
+        """Direct Qt deletion must release a mask parented to a surviving window."""
+        owner = QWidget()
+        owner.show()
+        processQtEvents()
+
+        try:
+            for _ in range(30):
+                messageBox = AppQMessageBox(parent=owner, text='Lifetime probe')
+                messageBox.open()
+                mask = messageBox._windowMask
+
+                self.assertIsNotNone(mask)
+
+                messageBox.deleteLater()
+                processQtEvents()
+
+                self.assertFalse(isValid(messageBox))
+                self.assertFalse(isValid(mask))
+                self.assertEqual(owner.findChildren(_AppMessageBoxMask), [])
+        finally:
+            owner.deleteLater()
+            processQtEvents()
+
     def testMessageBoxAndParentMaskHaveTransientOwnership(self):
         """Remove every parent event filter/mask over repeated modal presentation."""
         iterations = 60

tests/test_service_runtime.py

@@ -37,7 +37,7 @@ from PySide6.QtWidgets import QWidget
 
 from shiboken6 import isValid
 
-from tests.support import application, collectAtBoundary, waitFor
+from tests.support import processQtEvents, application, collectAtBoundary, waitFor
 
 from types import SimpleNamespace
 from unittest.mock import patch
@@ -259,6 +259,31 @@ class PluginNavigationManagerTest(unittest.TestCase):
         """Create the process-wide headless QApplication."""
         application()
 
+    def testDestroyedFactoryResultsAreRejectedBeforeRegistration(self):
+        """A retained Python wrapper is not proof of a valid plugin page."""
+        for pageType in (QtCore.QObject, QWidget):
+            with self.subTest(pageType=pageType):
+                page = pageType()
+                page.deleteLater()
+                processQtEvents()
+
+                provider = _NavigationProvider()
+                provider._invalidPage = lambda parent=None: page
+                host = _NavigationHost()
+                manager = PluginNavigationManager(_NavigationRegistry(provider))
+
+                try:
+                    with self.assertLogs(
+                        'Furious.Service.PluginUIManager', level='ERROR'
+                    ):
+                        pages = manager.registerPages(host)
+
+                    self.assertEqual(len(pages), 1)
+                    self.assertEqual(len(host.registrations), 1)
+                finally:
+                    host.deleteLater()
+                    processQtEvents()
+
     def testRegistrationIsIdempotentAndDeletesInvalidQObject(self):
         """Construct each descriptor once and destroy rejected Qt objects."""
         provider = _NavigationProvider()
@@ -296,7 +321,12 @@ class ConnectivityManagerTest(unittest.TestCase):
         reply = object()
         manager._testingEnabled = True
 
-        with patch.object(manager, 'webGET', return_value=reply) as webGet:
+        with (
+            patch.object(manager, 'webGET', return_value=reply) as webGet,
+            patch(
+                'Furious.Service.ConnectivityManager.AppSettings.get', return_value=None
+            ),
+        ):
             manager.startSingleTest()
             manager.startSingleTest()
 

tests/test_theme_transition.py

@@ -87,6 +87,53 @@ class ThemeTransitionTest(unittest.TestCase):
             QtCore.Qt.FindChildOption.FindDirectChildrenOnly,
         )
 
+    def testWindowDestructionReleasesCoordinatorAnimationState(self):
+        """Deleting a target must not leave its stopped animation registered."""
+        windows = []
+        transition = self.createTransition(windows, duration=100000)
+
+        for _ in range(30):
+            window = QWidget()
+            window.show()
+            processQtEvents()
+
+            windows[:] = [window]
+
+            transition.apply(lambda: None)
+            animation = next(iter(transition._animations))
+
+            window.deleteLater()
+            processQtEvents()
+
+            self.assertFalse(isValid(animation))
+            self.assertEqual(transition._animations, {})
+            self.assertEqual(transition._animationsByWindow, {})
+
+    def testCoordinatorDestructionReleasesWindowOwnedOverlays(self):
+        """Destroying the animation owner must also remove its snapshots."""
+        window = self.createWindow()
+
+        for _ in range(30):
+            owner = QtCore.QObject()
+            transition = ThemeTransition(
+                owner,
+                duration=100000,
+                windowProvider=lambda: (window,),
+                animationsEnabled=lambda: True,
+            )
+
+            transition.apply(lambda: None)
+            overlays = self.overlays(window)
+
+            self.assertEqual(len(overlays), 1)
+
+            owner.deleteLater()
+            processQtEvents()
+
+            self.assertFalse(isValid(transition))
+            self.assertFalse(isValid(overlays[0]))
+            self.assertEqual(self.overlays(window), [])
+
     def testThemeIsAppliedImmediatelyThenSnapshotCompletesAndIsRemoved(self):
         """Keep destination state live beneath one real fading snapshot."""
         window = self.createWindow()