From 1e4c8b6683adb5c964df46b42eaeb2d5815d0d71 Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Sat, 10 Oct 2026 21:09:04 +0800 Subject: [PATCH] Improve cleanup lifecycle readability Signed-off-by: Loren Eteval --- Furious/Application/DesktopApplication.py | 7 +++++ Furious/Controllers/ConnectionController.py | 35 +++++++++++++++++++++ Furious/Frozenlib/SingTUNRouting.py | 5 +++ Furious/Service/ConnectionManager.py | 22 +++++++++++++ Furious/Service/SingTUNHost.py | 7 +++++ tests/test_architecture_refactors.py | 3 ++ tests/test_runtime_lifecycle.py | 7 +++++ tests/test_sing_tun.py | 29 +++++++++++++++++ tests/test_sing_tun_linux_host.py | 6 ++++ 9 files changed, 121 insertions(+) diff --git a/Furious/Application/DesktopApplication.py b/Furious/Application/DesktopApplication.py index a5da1e99..de19d356 100644 --- a/Furious/Application/DesktopApplication.py +++ b/Furious/Application/DesktopApplication.py @@ -1033,12 +1033,16 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): '_connectionShutdownFinished', sender=self.connectionController, ) + try: self.connectionController.shutdown() except Exception as ex: # Any non-exit exceptions + logger.error(f'connection shutdown prevented application exit: {ex}') + self._connectionShutdownFinished(False) + return self._completeExit() @@ -1046,6 +1050,7 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): def _connectionShutdownFinished(self, success): if not self._exitRequested or self._exitCommitted: return + if success: singleShotWeakly(0, self, '_completeExit') else: @@ -1055,7 +1060,9 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): def _completeExit(self): if not self._exitRequested or self._exitCommitted: return + self._exitCommitted = True + QtCore.QCoreApplication.exit(self._exitCode) def run(self): diff --git a/Furious/Controllers/ConnectionController.py b/Furious/Controllers/ConnectionController.py index 42f6e555..0d1db5ba 100644 --- a/Furious/Controllers/ConnectionController.py +++ b/Furious/Controllers/ConnectionController.py @@ -224,9 +224,12 @@ class ConnectionController(QtCore.QObject): self._pendingHttpProxy = '' self.progressFinished.emit(True) + if not isValid(self): return + self._setActiveProfile(None) + if not isValid(self): return @@ -589,6 +592,7 @@ class ConnectionController(QtCore.QObject): if self.isDisconnecting(): self._reconnectAfterStop = False return False + if ( self.state is ConnectionState.Disconnected and not self._startAdmissionPending @@ -610,8 +614,10 @@ class ConnectionController(QtCore.QObject): self._startOperation = None self._setState(ConnectionState.Disconnecting) + if not isValid(self) or generation != self._connectionGeneration: return True + self._actionTimer.stop() if operation is not None and isValid(operation): @@ -637,9 +643,11 @@ class ConnectionController(QtCore.QObject): self._disconnectionNotification = notification asyncStop = getattr(self._coreManager, 'stopAsync', None) + if callable(asyncStop): try: self._stopOperation = stopOperation = asyncStop() + connectWeakly( stopOperation.finished, self, @@ -652,13 +660,19 @@ class ConnectionController(QtCore.QObject): '_connectionStopDestroyed', sender=stopOperation, ) + stopOperation.start() + return True except Exception as ex: # Any non-exit exceptions + self._runtimeCleanupFailed = True + logger.error(f'failed to stop connection runtime: {ex}') + self._finishDisconnection(generation, notification) + return True try: @@ -678,10 +692,14 @@ class ConnectionController(QtCore.QObject): def _connectionStopFinished(self, operation, error): if operation is not self._stopOperation: return + self._stopOperation = None + if error: self._runtimeCleanupFailed = True + logger.error(f'failed to stop connection runtime: {error}') + self._finishDisconnection( self._disconnectionGeneration, self._disconnectionNotification ) @@ -690,15 +708,19 @@ class ConnectionController(QtCore.QObject): if self._stopOperation is not None and not isValid(self._stopOperation): self._stopOperation = None self._runtimeCleanupFailed = True + self._finishDisconnection( self._disconnectionGeneration, self._disconnectionNotification ) def _finishDisconnection(self, generation, notification): reconnect, self._reconnectAfterStop = self._reconnectAfterStop, False + self._emitRuntimesChanged() + if not isValid(self): return + self._reset() if not isValid(self) or generation != self._connectionGeneration: @@ -713,22 +735,28 @@ class ConnectionController(QtCore.QObject): pass Mixins.ConnectionAware.callDisconnectedCallback() + if not isValid(self) or generation != self._connectionGeneration: return if notification: self.notificationRequested.emit(notification) + if not isValid(self): return if self._shuttingDown: if self._shutdownConnectPreference: AppSettings.turnON_('Connect') + success = not self._runtimeCleanupFailed + self._shutdownComplete = success + if not success: self._shuttingDown = False self._shutdownConnectPreference = None + self.shutdownFinished.emit(success) elif ( reconnect @@ -761,10 +789,14 @@ class ConnectionController(QtCore.QObject): """Stop runtime resources without changing the next-start preference.""" if self._shutdownComplete: self.shutdownFinished.emit(True) + return True + reconnectOnStartup = AppSettings.isStateON_('Connect') + if self._shutdownConnectPreference is None: self._shutdownConnectPreference = reconnectOnStartup + reconnectOnStartup = self._shutdownConnectPreference self._shuttingDown = True @@ -789,12 +821,15 @@ class ConnectionController(QtCore.QObject): self._runtimeCleanupFailed = False self._shutdownComplete = True self.shutdownFinished.emit(True) + return True except Exception: # Any non-exit exceptions + self._shuttingDown = False self._shutdownConnectPreference = None self.shutdownFinished.emit(False) + raise finally: if reconnectOnStartup: diff --git a/Furious/Frozenlib/SingTUNRouting.py b/Furious/Frozenlib/SingTUNRouting.py index 84aa9234..98a28168 100644 --- a/Furious/Frozenlib/SingTUNRouting.py +++ b/Furious/Frozenlib/SingTUNRouting.py @@ -416,6 +416,7 @@ class SingTUNLinuxRouting: """Report loss of an authorized helper while forwarding is active.""" if self._holding and self._ready and self._process.poll() is not None: return 'sing-tun privileged helper exited unexpectedly' + return '' def drain(self, *, wait=True): @@ -449,6 +450,7 @@ class SingTUNLinuxRouting: self._directory = None self._holding = False self._ready = False + return True def execute(self, commands, *, preparing=False, guard=False, holding=False): @@ -509,6 +511,7 @@ class SingTUNLinuxRouting: if holding: ready = pathlib.Path(directory.name, 'ready') staging = pathlib.Path(directory.name, 'ready.tmp') + lines.extend( [ 'printf "%s\\n" "$createdIndex" > ' + shlex.quote(str(staging)), @@ -587,6 +590,7 @@ class SingTUNLinuxRouting: """Pair successful additions with exact deletions in the same helper.""" name = shlex.quote(self.tun['Name']) index = shlex.quote('/sys/class/net/' + self.tun['Name'] + '/ifindex') + lines = [ 'createdIndex=; rulesAdded=0', 'cleanup() {', @@ -618,6 +622,7 @@ class SingTUNLinuxRouting: "trap 'exit 1' HUP INT TERM", ] ) + return lines def prepare(self, dnsAddress='1.1.1.1'): diff --git a/Furious/Service/ConnectionManager.py b/Furious/Service/ConnectionManager.py index dbc0e7f5..c8c2d6eb 100644 --- a/Furious/Service/ConnectionManager.py +++ b/Furious/Service/ConnectionManager.py @@ -1379,8 +1379,10 @@ class ConnectionStartOperation(QtCore.QObject): def _cleanupDestroyedStopOperation(reference, *_args): operation = reference() + if operation is not None: operation._terminal = True + if operation.manager._activeStopOperation is operation: operation.manager._activeStopOperation = None @@ -1392,11 +1394,15 @@ class ConnectionStopOperation(QtCore.QObject): def __init__(self, manager): super().__init__() + self.manager = manager self._terminal = False + self._timer = QtCore.QTimer(self) self._timer.setInterval(100) + connectWeakly(self._timer.timeout, self, '_poll') + self.destroyed.connect( functools.partial(_cleanupDestroyedStopOperation, weakref.ref(self)) ) @@ -1404,22 +1410,27 @@ class ConnectionStopOperation(QtCore.QObject): def start(self): if self._terminal or self._timer.isActive(): return + self._timer.start() self._poll() def _poll(self): if self._terminal: return + try: complete = self.manager._pollPendingReleases() except Exception as ex: # Any non-exit exceptions + if isValid(self): self._finish(str(ex) or type(ex).__name__) + return if not isValid(self) or self._terminal: return + if complete: self._finish('') elif any(lease.releaseError for lease in self.manager._pendingReleases): @@ -1428,8 +1439,10 @@ class ConnectionStopOperation(QtCore.QObject): def _finish(self, error): self._terminal = True self._timer.stop() + if self.manager._activeStopOperation is self: self.manager._activeStopOperation = None + try: self.finished.emit(self, error) finally: @@ -1602,6 +1615,7 @@ class ConnectionManager(Mixins.CleanupOnExit): """ if self._activeStopOperation is not None and isValid(self._activeStopOperation): raise RuntimeError('Connection runtime cleanup is incomplete') + self._retryPendingReleases() self._lastStartError = '' @@ -1643,6 +1657,7 @@ class ConnectionManager(Mixins.CleanupOnExit): """Create and schedule one manager-owned startup transaction.""" if self._activeStopOperation is not None and isValid(self._activeStopOperation): raise RuntimeError('Connection runtime cleanup is incomplete') + if self._activeStartOperation is not None: self._activeStartOperation.cancel() @@ -2263,23 +2278,30 @@ class ConnectionManager(Mixins.CleanupOnExit): operation = ConnectionStopOperation(self) except BaseException: self._retainStopLeases() + raise + self._activeStopOperation = operation + try: self._retainStopLeases() self.cancelStart() except BaseException: self._retainStopLeases() self._activeStopOperation = None + operation.deleteLater() + raise singleShotWeakly(0, operation, 'start') + return operation def _retainStopLeases(self): self._pendingReleases.extend(reversed(self._leases)) self._leases.clear() + for lease in self._pendingReleases: lease.router.beginRelease() diff --git a/Furious/Service/SingTUNHost.py b/Furious/Service/SingTUNHost.py index adfa05db..99d60f25 100644 --- a/Furious/Service/SingTUNHost.py +++ b/Furious/Service/SingTUNHost.py @@ -188,12 +188,14 @@ class SingTUNHostPlan: self._done.set() self._worker = threading.Thread(target=run, name='sing-tun-host') + try: self._worker.start() except BaseException: self._worker = None self._cleanupMethod = None self._done.set() + raise @property @@ -223,6 +225,7 @@ class SingTUNHostPlan: if self._worker.is_alive(): if self.platform == 'Linux': return False + raise RuntimeError('sing-tun host preparation is still stopping') self._worker = None @@ -585,6 +588,7 @@ class SingTUNHostPlan: or (self._linux is not None and self._linux.dnsTouched) ): return self._finishCleanupWork('restoreDNS') + return True def _finishCleanupWork(self, method, *args): @@ -599,9 +603,11 @@ class SingTUNHostPlan: if self._worker.is_alive(): if self.platform == 'Linux': return False + raise RuntimeError('sing-tun host restoration is still running') completedMethod = self._cleanupMethod + self._worker = None self._cleanupMethod = None @@ -610,6 +616,7 @@ class SingTUNHostPlan: if completedMethod is not None and completedMethod != method: return self._finishCleanupWork(method, *args) + return True def _recoverNative(self): diff --git a/tests/test_architecture_refactors.py b/tests/test_architecture_refactors.py index bf4d2a61..9ffbbba8 100644 --- a/tests/test_architecture_refactors.py +++ b/tests/test_architecture_refactors.py @@ -1175,14 +1175,17 @@ class ApplicationLifecycleTransactionTest(TestCase): eventFilter = CloseFilter(window) window.installEventFilter(eventFilter) + try: window.show() processQtEvents() + with mock.patch( 'Furious.Application.DesktopApplication.AppQMessageBox', messageType, ): window.close() + self.assertTrue(window.isVisible()) owner.exit.assert_called_once_with() finally: diff --git a/tests/test_runtime_lifecycle.py b/tests/test_runtime_lifecycle.py index 336d7ce7..2d82931a 100644 --- a/tests/test_runtime_lifecycle.py +++ b/tests/test_runtime_lifecycle.py @@ -111,16 +111,20 @@ class RuntimeLifecycleTest(TestCase): manager = ConnectionManager() runtime = _Runtime() manager._leases.append(RuntimeLease(runtime, RuntimeEventRouter())) + operation = manager.stopAsync() operation.finished.connect(lambda current, _error: deleteQObject(current)) + with mock.patch('sys.excepthook') as qtErrors: operation.start() processQtEvents() + qtErrors.assert_not_called() self.assertFalse(isValid(operation)) self.assertFalse(isValid(operation._timer)) self.assertIsNone(manager._activeStopOperation) self.assertEqual(manager._pendingReleases, []) + manager.cleanup() def testFailedStopObserverConstructionKeepsRuntimesOwnedAndBlocksReplacement(self): @@ -128,16 +132,19 @@ class RuntimeLifecycleTest(TestCase): runtime = _Runtime() lease = RuntimeLease(runtime, RuntimeEventRouter()) manager._leases.append(lease) + with mock.patch( 'Furious.Service.ConnectionManager.ConnectionStopOperation', side_effect=RuntimeError('observer factory'), ): with self.assertRaisesRegex(RuntimeError, 'observer factory'): manager.stopAsync() + self.assertEqual(manager.runtimes, []) self.assertEqual(manager._pendingReleases, [lease]) self.assertEqual(lease.state, RuntimeLeaseState.Releasing) self.assertIsNone(manager._activeStopOperation) + manager.cleanup() def testFailedLeaseReleaseRemainsOwnedUntilSuccessfulRetry(self): diff --git a/tests/test_sing_tun.py b/tests/test_sing_tun.py index 73f45e69..c28246dd 100644 --- a/tests/test_sing_tun.py +++ b/tests/test_sing_tun.py @@ -1344,38 +1344,47 @@ class SingTUNChildTest(unittest.TestCase): controllers = importlib.import_module( 'Furious.Controllers.ConnectionController' ) + plan = SingTUNHostPlan({}, platform='Linux') tun = plan.configuration['tun_options'] tun['Name'] = 'utun101' plan._linux = helper = SingTUNLinuxRouting(tun, 123456, 2000, threading.Event()) helper.index = 77 helper.dnsTouched = plan._activated = True + entered, authorized = threading.Event(), threading.Event() present, refused, processes = [True], [denied], [] def command(arguments): if not present[0]: return '[]' + if arguments == ['ip', '-N', '-j', 'link', 'show']: return '[{"ifname":"utun101","ifindex":77}]' if arguments == ['ip', '-4', '-N', '-j', 'rule', 'show']: return '[{"priority":2001,"src":"all","table":123456,"suppress_prefixlen":0}]' + return '[]' def launch(arguments, **kwargs): self.assertEqual(arguments[:2], ['pkexec', 'bash']) + process = mock.Mock() process.poll.return_value = None def wait(timeout=None): self.assertIsNone(timeout) entered.set() + if not authorized.wait(10): raise RuntimeError('fixture authorization was not released') + code = 126 if refused[0] else 0 process.poll.return_value = code + if not code: present[0] = False + return code process.wait.side_effect = wait @@ -1401,10 +1410,12 @@ class SingTUNChildTest(unittest.TestCase): router.attach(runtime, manager) lease = RuntimeLease(runtime, router, applicationTUN=True) manager._leases.append(lease) + controller = ConnectionController( coreManager=manager, updatesManager=mock.Mock() ) controller._state = ConnectionState.Connected + try: yield SimpleNamespace( controller=controller, @@ -1422,12 +1433,15 @@ class SingTUNChildTest(unittest.TestCase): finally: refused[0] = False authorized.set() + operation = manager.stopAsync() operation.start() self.assertTrue(waitFor(lambda: operation._terminal, timeout=10)) + manager.cleanup() controller.deleteLater() processQtEvents() + qtErrors.assert_not_called() def testDisconnectAuthorizationLeavesQtResponsiveAndKeepsItsLease(self): @@ -1435,15 +1449,18 @@ class SingTUNChildTest(unittest.TestCase): controller, runtime = fixture.controller, fixture.runtime states, ticks = [], [] controller.stateChanged.connect(states.append) + self.assertTrue(controller.startDisconnection()) self.assertTrue(fixture.entered.wait(2)) QtCore.QTimer.singleShot(0, lambda: ticks.append(True)) self.assertTrue(waitFor(lambda: bool(ticks))) + self.assertEqual(controller.state, ConnectionState.Disconnecting) self.assertTrue(runtime.cleanupPending) self.assertFalse(runtime._control.closed) self.assertEqual(fixture.manager._pendingReleases, [fixture.lease]) self.assertEqual(len(fixture.processes), 1) + with self.assertRaisesRegex(RuntimeError, 'cleanup is incomplete'): fixture.manager.startAsync(None, '') @@ -1467,6 +1484,7 @@ class SingTUNChildTest(unittest.TestCase): fixture.controller.startDisconnection() self.assertTrue(fixture.entered.wait(2)) fixture.authorized.set() + self.assertTrue( waitFor( lambda: fixture.controller.state is ConnectionState.Disconnected @@ -1485,6 +1503,7 @@ class SingTUNChildTest(unittest.TestCase): self.assertTrue(fixture.entered.wait(2)) processQtEvents() reconnect.assert_not_called() + fixture.authorized.set() self.assertTrue(waitFor(lambda: reconnect.call_count == 1)) self.assertEqual(fixture.manager._pendingReleases, []) @@ -1493,8 +1512,10 @@ class SingTUNChildTest(unittest.TestCase): with self._delayedLinuxRecovery() as fixture: fixture.controller.startDisconnection() self.assertTrue(fixture.entered.wait(2)) + operation = fixture.controller._stopOperation deleteQObject(operation) + self.assertFalse(isValid(operation._timer)) self.assertIsNone(fixture.manager._activeStopOperation) self.assertEqual(fixture.manager._pendingReleases, [fixture.lease]) @@ -1506,12 +1527,14 @@ class SingTUNChildTest(unittest.TestCase): AppSettings.turnON_('Connect') results, ticks = [], [] fixture.controller.shutdownFinished.connect(results.append) + self.assertIs(fixture.controller.shutdown(), False) self.assertTrue(fixture.entered.wait(2)) QtCore.QTimer.singleShot(0, lambda: ticks.append(True)) self.assertTrue(waitFor(lambda: bool(ticks))) self.assertEqual(results, []) self.assertTrue(AppSettings.isStateON_('Connect')) + fixture.authorized.set() self.assertTrue(waitFor(lambda: results == [True])) self.assertTrue(AppSettings.isStateON_('Connect')) @@ -1522,11 +1545,13 @@ class SingTUNChildTest(unittest.TestCase): controllers = importlib.import_module( 'Furious.Controllers.ConnectionController' ) + entered, released = threading.Event(), threading.Event() plan = SingTUNHostPlan({}, platform='Linux') def prepare(_addresses): entered.set() + if not released.wait(5): raise RuntimeError('fixture preparation was not released') @@ -1541,6 +1566,7 @@ class SingTUNChildTest(unittest.TestCase): router.attach(runtime, manager) lease = RuntimeLease(runtime, router, applicationTUN=True) manager._leases.append(lease) + controller = ConnectionController( coreManager=manager, updatesManager=mock.Mock() ) @@ -1555,6 +1581,7 @@ class SingTUNChildTest(unittest.TestCase): self.assertTrue(waitFor(lambda: bool(ticks))) self.assertEqual(controller.state, ConnectionState.Disconnecting) self.assertEqual(manager._pendingReleases, [lease]) + released.set() self.assertTrue( waitFor(lambda: controller.state is ConnectionState.Disconnected) @@ -1563,9 +1590,11 @@ class SingTUNChildTest(unittest.TestCase): self.assertTrue(runtime._control.closed) finally: released.set() + cleanup = manager.stopAsync() cleanup.start() self.assertTrue(waitFor(lambda: cleanup._terminal)) + manager.cleanup() controller.deleteLater() processQtEvents() diff --git a/tests/test_sing_tun_linux_host.py b/tests/test_sing_tun_linux_host.py index 467f709c..d0ec0626 100644 --- a/tests/test_sing_tun_linux_host.py +++ b/tests/test_sing_tun_linux_host.py @@ -161,8 +161,10 @@ class SingTUNLinuxHostTest(unittest.TestCase): def wait(timeout=None): self.assertIsNone(timeout) entered.set() + if not authorized.wait(5): raise RuntimeError('fixture authorization was not released') + process.poll.return_value = 0 return 0 @@ -173,6 +175,7 @@ class SingTUNLinuxHostTest(unittest.TestCase): self.host.execute([['ip', 'link', 'delete', 'dev', 'utun101']]) except Exception as ex: # Any non-exit exceptions + errors.append(ex) finally: complete.set() @@ -182,6 +185,7 @@ class SingTUNLinuxHostTest(unittest.TestCase): ), mock.patch.object(self.module.subprocess, 'Popen', return_value=process): worker = threading.Thread(target=recover) worker.start() + try: self.assertTrue(entered.wait(2)) self.assertFalse(complete.is_set()) @@ -269,8 +273,10 @@ class SingTUNLinuxHostTest(unittest.TestCase): self.assertIs(self.host._process, process) self.assertTrue(directory.exists()) + process.poll.return_value = 126 self.host.drain() + self.assertFalse(directory.exists()) def testLiveForwarderDnsCannotBeReportedAsRestored(self):