From 3df3d03802750102e05eb0eda82aa0665a0ef795 Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Sun, 11 Oct 2026 15:54:08 +0800 Subject: [PATCH] Fix application shutdown Signed-off-by: Loren Eteval --- Furious/Application/AGENTS.md | 19 +- Furious/Application/DesktopApplication.py | 41 ++-- Furious/Service/AGENTS.md | 8 +- Furious/Utility/AGENTS.md | 24 ++- Furious/Utility/AppMainProcess.py | 64 ++++++- Furious/__main__.py | 4 +- tests/README.md | 27 ++- tests/fixtures/shutdown_probe.py | 220 ++++++++++++++++++++++ tests/test_application_process.py | 83 +++++++- tests/test_architecture_refactors.py | 3 + 10 files changed, 448 insertions(+), 45 deletions(-) create mode 100644 tests/fixtures/shutdown_probe.py diff --git a/Furious/Application/AGENTS.md b/Furious/Application/AGENTS.md index 17f055c8..83681ffa 100644 --- a/Furious/Application/AGENTS.md +++ b/Furious/Application/AGENTS.md @@ -17,23 +17,26 @@ relative to this source tree's root. `aboutToQuit` and the event-loop `finally` may both reach it; repeated entry must not repeat registered stages. One callback failure does not skip later stages, but the stack consumes that callback and does not retry it. Its successful `close()` return means this invocation ran the stack, not that every resource was released. - Service-level retry/retention obligations must be satisfied before the owner disappears. `exit()` first requests - controller shutdown while Qt delivery is still active, then requests Qt termination after successful connection - cleanup. Action/window/session handlers do not run the application cleanup stack directly. Pending or refused - quit must keep the main window usable; the application retains controllers whose shutdown has not completed. + `exit()` announces shutdown to `AppMainProcess` before requesting controller cleanup. Failed connection cleanup + is retried by one application-owned single-shot timer while Qt delivery remains active; successful cleanup requests + Qt termination. Repeated exit requests do not reset the deadline or requested result. Action/window/session handlers + do not run the application cleanup stack directly. The existing parent enforces the ten-second deadline even if + a synchronous cleanup stage blocks Qt, then forces application exit. Final cleanup announces shutdown too, covering + run-loop failure and direct Qt quit. This policy requires the supervised entry point; constructing an application + alone does not create a watchdog. Forced exit can interrupt unfinished persistence and host restoration. `tests/test_architecture_refactors.py` exercises the real quit/event-loop boundary alongside cleanup ordering. - A stage that fails before its cleanup callback is registered must release its own partial acquisitions. The outer cleanup stack releases completed stages; it cannot discover half-built controllers, UI, logging handlers, or native listeners. Keep each successful sub-acquisition reachable by the stage's failure cleanup before beginning the next fallible constructor; assigning several constructed owners together does not provide this guarantee. - Restore logging configuration as well as closing handlers. A failed controller shutdown currently retains that - controller, but the consumed application cleanup stage does not automatically retry it. Preserve the owner and - report incomplete cleanup; any retry policy must specify who invokes it before dependent owners are dismantled. + Restore logging configuration as well as closing handlers. Connection retries happen before the cleanup stack; + the consumed final cleanup stage does not retry arbitrary callbacks. Preserve incomplete resource ownership until + graceful release or forced process exit, and keep cleanup failures visible. For resource-owning controllers, schedule native deletion only after successful shutdown. UI deletion and workflow draining are separate stages; trace both rather than assuming `deleteLater()` waits for resources. Review cooperative pool drains separately from the cleanup stack's ordering guarantees. The application pool's timed wait logs unfinished work, whereas subscription preparation waits synchronously after its diagnostic timeout. - Neither policy can be inferred from reverse cleanup order or from the name of a shutdown method. + The outer deadline bounds application shutdown without forcibly terminating individual in-process threads. - Singleton election serializes cooperating candidates, re-probes after waiting, recovers only a confirmed stale endpoint, and fails closed when ownership is uncertain, including privilege handoff. A successful Windows local-server listen alone does not establish exclusivity; command delivery and endpoint ownership are separate diff --git a/Furious/Application/DesktopApplication.py b/Furious/Application/DesktopApplication.py index de19d356..8834dd9b 100644 --- a/Furious/Application/DesktopApplication.py +++ b/Furious/Application/DesktopApplication.py @@ -404,6 +404,7 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): """Represent application.""" _sessionShutdownRequested = QtCore.Signal() + shutdownRequested = QtCore.Signal(int) ThreadPoolShutdownTimeout = 5000 def __init__(self, argv): @@ -461,6 +462,10 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): self._exitCommitted = False self._exitConnection = None self._exitCode = ApplicationRunner.ExitCode.ExitSuccess.value + self._shutdownRetryTimer = QtCore.QTimer(self) + self._shutdownRetryTimer.setSingleShot(True) + self._shutdownRetryTimer.setInterval(500) + self._shutdownRetryTimer.timeout.connect(self._requestConnectionShutdown) # win32session invokes its callback on the native listener thread. # Queue the request so every Qt-owned cleanup stage stays on the GUI thread. @@ -965,6 +970,8 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): @QtCore.Slot() def cleanup(self): """Release every successfully acquired application stage exactly once.""" + self.shutdownRequested.emit(self._exitCode) + if self._cleanupStack.close(): logger.info('final cleanup done') @@ -1024,6 +1031,7 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): self._exitRequested = True self._exitCode = int(exitcode) self.setExitingFlag(True) + self.shutdownRequested.emit(self._exitCode) if self.connectionController is not None: if self._exitConnection is None: @@ -1034,34 +1042,41 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): 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) + self._requestConnectionShutdown() return self._completeExit() + def _requestConnectionShutdown(self): + """Retry failed connection cleanup within the supervisor's deadline.""" + if not self._exitRequested or self._exitCommitted: + return + + try: + self.connectionController.shutdown() + except Exception as ex: + # Any non-exit exceptions + + logger.error(f'connection shutdown prevented application exit: {ex}') + self._connectionShutdownFinished(False) + def _connectionShutdownFinished(self, success): if not self._exitRequested or self._exitCommitted: return if success: + self._shutdownRetryTimer.stop() singleShotWeakly(0, self, '_completeExit') else: - self._exitRequested = False - self.setExitingFlag(False) + self._shutdownRetryTimer.start() def _completeExit(self): if not self._exitRequested or self._exitCommitted: return self._exitCommitted = True + self._shutdownRetryTimer.stop() QtCore.QCoreApplication.exit(self._exitCode) @@ -1116,12 +1131,14 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): if self._exitCommitted: return self._exitCode - return self.exec() + self._exitCode = self.exec() + return self._exitCode except Exception: # Any non-exit exceptions traceback.print_exc() - return ApplicationRunner.ExitCode.UnknownException.value + self._exitCode = ApplicationRunner.ExitCode.UnknownException.value + return self._exitCode finally: self.cleanup() diff --git a/Furious/Service/AGENTS.md b/Furious/Service/AGENTS.md index e6647ea1..727be9ef 100644 --- a/Furious/Service/AGENTS.md +++ b/Furious/Service/AGENTS.md @@ -113,7 +113,9 @@ source tree's root. Worker-safe payload import and reconciliation preparation run in the manager's bounded pool over copied data; unclassified plugin parsers stay on the GUI compatibility path. The manager's synchronous shutdown closes admission, cancels work, and retains the pool/relay until workers finish. A slow-shutdown warning is diagnostic, - not a deadline that permits destroying running workers; a non-returning plugin can still block shutdown. + not a deadline that permits destroying running workers. A non-returning plugin can block this local drain; + during application shutdown the existing parent process enforces the application deadline and kills the child. + That fallback does not establish successful service cleanup or make ordinary owner destruction non-blocking. Subscription preparation workers never read live repositories or Qt models. The GUI thread verifies the full source signature and group revision, commits while preserving live profile identity/local metadata, then publishes coalesced status/structure. @@ -157,8 +159,8 @@ source tree's root. History contains finite values for registered metrics on a monotonic timeline. A missing metric sample is not a measured zero; preserve that distinction when adding providers or aggregating sparse series. Closing executor admission or cancelling a future does not terminate a query already inside plugin code; test stale-result suppression - separately from worker completion. Monitor contracts must bound blocking work; report non-returning providers as a - shutdown limitation, not successful cancellation. + separately from worker completion. Monitor contracts must bound blocking work; application forced exit can end a + stuck worker but is not successful cooperative cancellation or proof of external-resource cleanup. ## Profile testing diff --git a/Furious/Utility/AGENTS.md b/Furious/Utility/AGENTS.md index 2457d320..63a11a2d 100644 --- a/Furious/Utility/AGENTS.md +++ b/Furious/Utility/AGENTS.md @@ -7,7 +7,7 @@ source tree's root. - `Utility` owns the child-side wrapper used by the outer application process and crash/exit translation. It is not a miscellaneous helper namespace and does not own application composition, repositories, runtimes, or UI policy. -- `AppMainProcess` owns one exact Qt application child and one small synchronized crash-log result. Do not add a +- `AppMainProcess` owns one exact Qt application child and small shared shutdown/crash scalars. Do not add a `multiprocessing.Manager` or auxiliary child merely to communicate status, and preserve the platform’s explicit spawn behavior. - Required exception reporting covers failures before and after application construction. The child runs the supplied @@ -20,19 +20,23 @@ source tree's root. control flow. A partially constructed QApplication may lack its log manager; save the traceback without assuming full composition. Spawned cases in `tests/test_application_process.py` verify classification, original diagnostics, the shared crash flag, unavailable crash output and intentional exits, separately from direct hook mapping. -- The parent entry point joins only the child it created and shows the fallback Qt report only for a nonzero result. - That join follows the GUI session lifetime; it is not a short startup-readiness deadline. Tests must bound their - own waits and reap their exact child if the fixture fails. A child stuck in cooperative worker cleanup can - therefore keep the parent join pending; there is no general shutdown watchdog here. Do not turn a test timeout - into an undocumented production kill policy. Never discover or terminate processes by name, and keep - normal/source/packaged command-line entry points equivalent. - Application-child shutdown and each proxy/TUN runtime's release are separate boundaries. The outer join cannot - establish that an inner service completed cleanup; trace its exact resource owner before changing supervision. -- Shared crash status is a synchronized Boolean plus the child's semantic exit result; set the flag only after the +- The parent entry point waits through `waitForExit()` for only the child it created. The child's shutdown signal, + exception hook, signal handler, or runner completion arms one monotonic deadline before final cleanup or interpreter + finalization. Ordinary operation has no deadline; repeated shutdown notifications cannot extend the grace period. + After ten seconds the parent kills and boundedly reaps that exact child, even if Qt or Python's GIL is blocked. + Keep this watchdog in the existing parent: a timer/thread in the child cannot enforce that contract. Never + discover or terminate processes by name, and keep normal/source/packaged entry points equivalent. + Forced exit preserves the requested semantic result, so an intentional quit does not open a crash dialog solely + because the OS reports a killed process. The fallback report still handles nonzero semantic results. + Forced application exit is not proof of runtime/helper termination, saved data, or host-network restoration; + remaining cleanup may be interrupted. `tests/fixtures/shutdown_probe.py` proves exact child reaping for stuck + workers, cleanup and GIL-holding calls, alongside normal and recovered cleanup. Tests retain their own failsafe. +- Shared crash status is a lock-free Boolean plus the child's semantic exit result; set the flag only after the diagnostic file is written successfully. Text may include retained logs plus a traceback, so the Boolean channel does not bound the crash file's size or sanitize its contents. Keep crash-write failure separate from the primary exit result: the flag proves only that a file write completed, not that the child succeeded or the report can be parsed as an exit protocol. Test reporting both with and without a constructed application/log manager. + Shutdown scalars are also lock-free so killing a child cannot strand the parent on a lock held by that child. Preserve the child's original exit classification even when writing or opening the secondary crash report fails; report presentation is not permission to restart the application or its managed runtimes. - Fallback presentation runs in the parent after a nonzero child result. It constructs a Qt application for the diff --git a/Furious/Utility/AppMainProcess.py b/Furious/Utility/AppMainProcess.py index 63114041..92406fdb 100644 --- a/Furious/Utility/AppMainProcess.py +++ b/Furious/Utility/AppMainProcess.py @@ -26,6 +26,7 @@ from typing import Callable import os import sys +import time import signal import logging import datetime @@ -47,19 +48,58 @@ else: class AppMainProcess(ProcessContext.Process): """Represent app main process.""" + ShutdownTimeout = 10.0 + ShutdownPollInterval = 0.1 + KillJoinTimeout = 3.0 + def __init__(self, func: Callable[[], ApplicationRunner], **kwargs): """Initialize the AppMainProcess.""" super().__init__(**kwargs) self.startupTime = str(datetime.datetime.now()).replace(':', '') self.logFileName = f'{self.startupTime}.log' - # One synchronized scalar is sufficient for the parent/child result. - # A multiprocessing.Manager would create a second child process whose - # lifecycle is unrelated to the exact application process we own. - self.fileWritten = ProcessContext.Value('b', False) + # Shared scalars need neither a manager process nor a response from the + # child's potentially blocked event loop during shutdown. + self.fileWritten = ProcessContext.Value('b', False, lock=False) + self._shutdownDeadline = ProcessContext.Value('d', 0.0, lock=False) + self._shutdownExitCode = ProcessContext.Value('i', 0, lock=False) self.func = func self.application = None + self.forcedExit = False + + def beginShutdown(self, exitcode=None): + """Publish the semantic result and arm one deadline before cleanup starts.""" + if exitcode is not None: + self._shutdownExitCode.value = int(exitcode) + + if not self._shutdownDeadline.value: + self._shutdownDeadline.value = time.monotonic() + self.ShutdownTimeout + + def waitForExit(self): + """Join this child, forcing exit once its shutdown grace period expires.""" + while self.is_alive(): + deadline = self._shutdownDeadline.value + + if deadline and time.monotonic() >= deadline: + exitcode = self._shutdownExitCode.value + self.forcedExit = True + logger.warning('application shutdown timed out; forcing process exit') + + # terminate() is SIGTERM on POSIX, which this child handles by + # requesting graceful shutdown again. kill() cannot be caught. + self.kill() + self.join(self.KillJoinTimeout) + + if self.is_alive(): + raise RuntimeError('application process survived forced shutdown') + + return exitcode + + self.join(self.ShutdownPollInterval) + + self.join(0) + return self.exitcode def exceptHook(self, exceptionType, exceptionValue, tb, *, raiseSystemExit=False): """Handle except hook for the app main process.""" @@ -71,6 +111,7 @@ class AppMainProcess(ProcessContext.Process): else: exitcode = ApplicationRunner.ExitCode.UnknownException.value + self.beginShutdown(exitcode) logger.error(f'stopped with exitcode {exitcode}') self.saveCrashLog(exceptionType, exceptionValue, tb) @@ -118,6 +159,7 @@ class AppMainProcess(ProcessContext.Process): def handler(self, signum, frame): """Handle r.""" + self.beginShutdown() logger.info(f'received signal {signal.Signals(signum).name}. Exit application') if self.application is None: @@ -132,10 +174,22 @@ class AppMainProcess(ProcessContext.Process): try: self.application = self.func() + shutdownRequested = getattr(self.application, 'shutdownRequested', None) + + if shutdownRequested is not None: + shutdownRequested.connect(self.beginShutdown) + for sig in [signal.SIGTERM, signal.SIGINT]: signal.signal(sig, self.handler) - sys.exit(self.application.run()) + exitcode = self.application.run() + self.beginShutdown(exitcode) + sys.exit(exitcode) + except SystemExit as ex: + self.beginShutdown( + ex.code if isinstance(ex.code, int) else (0 if ex.code is None else 1) + ) + raise except Exception: # Any non-exit exceptions diff --git a/Furious/__main__.py b/Furious/__main__.py index 525f6bda..63ddce0c 100644 --- a/Furious/__main__.py +++ b/Furious/__main__.py @@ -61,9 +61,7 @@ def runAppMain(): process = AppMainProcess(functools.partial(DesktopApplication, sys.argv)) process.start() - process.join() - - exitcode = process.exitcode + exitcode = process.waitForExit() if exitcode == 0: sys.exit(exitcode) diff --git a/tests/README.md b/tests/README.md index a0791ce4..7f0a677b 100644 --- a/tests/README.md +++ b/tests/README.md @@ -100,7 +100,7 @@ worker. Choose tests by the changed contract rather than by filename alone. | Module | Coverage | | --- | --- | -| [test_application_process.py](test_application_process.py) | Exact application-child ownership, spawned factory/runner failure classification and crash reporting, captured and asserted expected child diagnostics, partial Qt startup, crash-write failure, intentional exits, exception/signal handling and command-line dispatch. | +| [test_application_process.py](test_application_process.py) | Exact application-child ownership, spawned factory/runner failure classification and crash reporting, partial Qt startup, crash-write failure, intentional exits, exception/signal handling and command-line dispatch; real-child shutdown deadlines for stuck workers/cleanup/GIL, cleanup retries, normal exit and semantic results after forced termination. | | [test_architecture_refactors.py](test_architecture_refactors.py) | Startup acquisition/rollback including partial controller construction and cleanup retry, singleton election and real isolated IPC race, tray/exit policy, host integration ownership, bounded core-log transport, connection transactions, stylesheet composition. | | [test_connection_startup_async.py](test_connection_startup_async.py) | Real local-listener readiness, timeout/cancel/replacement, semantic exits, DNS reply lifetime, mocked platform-specific TUN sequencing, quoted Linux tun2socks scripts and unchanged route-existence detection in both startup paths. | | [test_controllers.py](test_controllers.py) | Connection state/error/reconnect transitions, reentrant cancellation/replacement before launch and during completion, startup restoration, shared settings, routing fallback persistence and tray/selector agreement after custom-routing disable/re-enable. | @@ -280,6 +280,10 @@ must capture and assert callback errors as well. Cancellation, logical completion, and resource release are different assertions. For example, subscription Stop Updates rejects late work but running preparation remains owned until it returns. Its slow-shutdown warning is not an exit deadline. +Application shutdown has a separate ten-second deadline enforced by the parent +process. The shutdown probe uses shorter deadlines and verifies actual child +reaping, including blocked Qt cleanup and interpreter executor finalization. +Forced application exit does not prove external runtimes or host state were cleaned. The profile-test shutdown regression checks independent Tcping/download cleanup and a later retry while a Ping worker is gated; it does not prove arbitrary third-party work can always be forcibly stopped. @@ -290,6 +294,27 @@ reported skips. Re-enable them together with the feature. ## Standalone benchmarks and fixtures +[fixtures/shutdown_probe.py](fixtures/shutdown_probe.py) exercises the supervised +application with isolated settings and mocked host effects. Run it in source or +compile it with Nuitka's PySide6 plugin, Furious package data and the harness's +`tests/fixtures/offscreen.json` data file: + +```text +python -m tests.fixtures.shutdown_probe subscription +python -m tests.fixtures.shutdown_probe statistics +python -m tests.fixtures.shutdown_probe cleanup +python -m tests.fixtures.shutdown_probe gil +python -m tests.fixtures.shutdown_probe runtime-stuck +python -m tests.fixtures.shutdown_probe runtime-retry +python -m tests.fixtures.shutdown_probe startup-failure +python -m tests.fixtures.shutdown_probe normal +``` + +JSON reports include actual child liveness, forced-exit classification, semantic +result, deadline overrun and cleanup events. The unittest assertions check these +outcomes; standalone invocations provide the same evidence for compiled checks. +A test-only failsafe reaps the exact child if production supervision regresses. + [benchmarks/benchmark_log_manager.py](benchmarks/benchmark_log_manager.py) measures steady append, core rollover, category/global snapshots, and retention-heavy append using the current checkout. Save a baseline before a change, then compare diff --git a/tests/fixtures/shutdown_probe.py b/tests/fixtures/shutdown_probe.py new file mode 100644 index 00000000..0582f25d --- /dev/null +++ b/tests/fixtures/shutdown_probe.py @@ -0,0 +1,220 @@ +# Copyright (C) 2024–present Loren Eteval & contributors +# +# This file is part of Furious. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Exercise bounded application shutdown in a real, isolated supervised child.""" + +from __future__ import annotations + +from tests.support import _initializeSettingsSandbox + +_initializeSettingsSandbox() + +from Furious.Application import DesktopApplication +from Furious.Controllers import ConnectionController, ConnectionState +from Furious.Interface import CoreRuntime, RuntimeState +from Furious.Plugins import TrafficCounters, TrafficStatsMonitor +from Furious.Repository import Storage +from Furious.Service.ConnectionManager import ConnectionManager +from Furious.Service.RuntimeLease import RuntimeEventRouter, RuntimeLease +from Furious.Service.SubscriptionManager import SubscriptionManager +from Furious.Service.TrafficStatsManager import TrafficStatsManager +from Furious.Utility.AppMainProcess import AppMainProcess + +from PySide6 import QtCore + +from pathlib import Path +from types import SimpleNamespace +from unittest import mock + +import argparse +import ctypes +import functools +import json +import multiprocessing +import os +import sys +import tempfile +import threading +import time + + +def _mark(path, message): + with open(path, 'a', encoding='utf-8') as file: + file.write(message + '\n') + + +class _Runtime(CoreRuntime): + def __init__(self, mode, path): + super().__init__() + self.mode, self.path, self.attempts = mode, path, 0 + + @staticmethod + def name(): + return 'Shutdown fixture' + + @staticmethod + def version(): + return '1' + + def start(self): + self.setState(RuntimeState.Alive) + + def stop(self): + self.attempts += 1 + _mark(self.path, 'runtime stop') + if self.mode == 'runtime-stuck' or self.attempts < 3: + raise RuntimeError('fixture refuses cleanup') + self.setState(RuntimeState.Exited) + + +class _Application(DesktopApplication): + def __init__(self, mode, path, exitcode, idleMilliseconds): + super().__init__(['shutdown-probe']) + self.mode, self.path = mode, path + self.requestedCode, self.idleMilliseconds = exitcode, idleMilliseconds + self._shutdownRetryTimer.setInterval(25) + self.setQuitOnLastWindowClosed(False) + + def shouldExitForExistingInstance(self): + return False + + def addEnviron(self): + return SimpleNamespace(shutdown=self._pluginShutdown) + + def addStorage(self): + pass + + def addCustomFont(self): + pass + + def configureLogging(self): + pass + + def _logRuntimeInformation(self): + pass + + def _initializeThemeDetection(self): + pass + + def _initializeSystemIntegration(self): + pass + + def _initializeControllers(self): + manager = ConnectionManager() + if self.mode.startswith('runtime-'): + runtime = _Runtime(self.mode, self.path) + manager._leases.append(RuntimeLease(runtime, RuntimeEventRouter())) + self.connectionController = ConnectionController( + parent=self, coreManager=manager, updatesManager=mock.Mock() + ) + if self.mode.startswith('runtime-'): + self.connectionController._state = ConnectionState.Connected + + def _initializeUI(self): + if self.mode == 'startup-failure': + raise RuntimeError('fixture startup failure') + + if self.mode in ('subscription', 'statistics'): + started = threading.Event() + + def blocked(_target): + started.set() + threading.Event().wait() + return TrafficCounters(1, 2) + + if self.mode == 'subscription': + with mock.patch.object(Storage, 'UserSubs', return_value={}): + self.workerOwner = SubscriptionManager(self) + self.workerOwner._startPreparationJob('probe', {}, blocked) + self._cleanupStack.register( + 'subscription worker', self.workerOwner.shutdown + ) + else: + self.workerOwner = TrafficStatsManager(self) + self.workerOwner._activateMonitor(TrafficStatsMonitor(blocked, None)) + + assert started.wait(3), 'fixture worker did not start' + + QtCore.QTimer.singleShot(self.idleMilliseconds, self._requestQuit) + + def _requestQuit(self): + _mark(self.path, 'quit requested') + self.exit(self.requestedCode) + self.exit(42) + + def _pluginShutdown(self): + _mark(self.path, 'cleanup entered') + if self.mode in ('cleanup', 'startup-failure'): + threading.Event().wait() + elif self.mode == 'gil': + # PyDLL deliberately retains the GIL: a Python watchdog thread or + # Qt timer in this child cannot enforce the shutdown deadline. + if os.name == 'nt': + ctypes.PyDLL('kernel32').Sleep(60000) + else: + ctypes.PyDLL(None).sleep(60) + _mark(self.path, 'cleanup finished') + + def run(self): + with mock.patch('Furious.Controllers.ConnectionController.SystemProxy.off'): + return super().run() + + +def runShutdownProbe(mode, *, timeout=0.5, exitcode=0, idleMilliseconds=0): + """Return actual child reaping and exit classification, with a test failsafe.""" + with tempfile.TemporaryDirectory(prefix='furious-shutdown-') as directory: + path = str(Path(directory) / 'events.txt') + process = AppMainProcess( + functools.partial(_Application, mode, path, exitcode, idleMilliseconds) + ) + process.ShutdownTimeout = timeout + process.start() + + # Test-only backstop owns this exact child if the production deadline + # regresses. A broken deadline then fails assertions without an orphan. + def failSafe(): + if process.is_alive(): + process.kill() + + safety = threading.Timer(12, failSafe) + safety.daemon = True + safety.start() + try: + result = process.waitForExit() + return { + 'mode': mode, + 'exitcode': result, + 'forced': process.forcedExit, + 'alive': process.is_alive(), + 'deadlineOverrun': time.monotonic() - process._shutdownDeadline.value, + 'events': Path(path).read_text(encoding='utf-8').splitlines(), + } + finally: + safety.cancel() + safety.join(2) + if process.is_alive(): + process.kill() + process.join(3) + process.close() + + +if __name__ == '__main__': + multiprocessing.freeze_support() + parser = argparse.ArgumentParser() + parser.add_argument('mode') + arguments = parser.parse_args() + print(json.dumps(runShutdownProbe(arguments.mode))) diff --git a/tests/test_application_process.py b/tests/test_application_process.py index d8cd89e4..e2d32dfa 100644 --- a/tests/test_application_process.py +++ b/tests/test_application_process.py @@ -23,6 +23,7 @@ from Furious.Interface import ApplicationRunner from Furious.Utility.AppMainProcess import AppMainProcess from tests.support import application +from tests.fixtures.shutdown_probe import runShutdownProbe from contextlib import redirect_stderr from pathlib import Path @@ -118,6 +119,63 @@ def _exitingApplication(stage, crashDirectory): class AppMainProcessTest(unittest.TestCase): """Verify exact child ownership, crash mapping, and signal safety.""" + def testStuckWorkersAndCleanupCannotKeepTheApplicationAlive(self): + for mode in ('subscription', 'statistics', 'cleanup', 'gil', 'runtime-stuck'): + with self.subTest(mode=mode): + result = runShutdownProbe(mode) + self.assertTrue(result['forced'], result) + self.assertEqual(result['exitcode'], 0, result) + self.assertFalse(result['alive'], result) + self.assertLess(result['deadlineOverrun'], 3, result) + self.assertIn('quit requested', result['events']) + if mode == 'statistics': + self.assertIn('cleanup finished', result['events']) + elif mode == 'runtime-stuck': + self.assertGreater(result['events'].count('runtime stop'), 1) + + def testNormalAndRecoveredShutdownDoNotForceExit(self): + for mode in ('normal', 'runtime-retry'): + with self.subTest(mode=mode): + result = runShutdownProbe(mode, timeout=2, exitcode=23) + self.assertFalse(result['forced'], result) + self.assertEqual(result['exitcode'], 23, result) + self.assertIn('cleanup finished', result['events']) + if mode == 'runtime-retry': + self.assertGreaterEqual(result['events'].count('runtime stop'), 3) + + def testDeadlineDoesNotApplyUntilShutdownStarts(self): + result = runShutdownProbe('normal', timeout=0.3, idleMilliseconds=700) + self.assertFalse(result['forced'], result) + self.assertEqual(result['exitcode'], 0, result) + self.assertIn('quit requested', result['events']) + + def testStartupFailureArmsDeadlineBeforeBlockedRollback(self): + result = runShutdownProbe('startup-failure') + self.assertTrue(result['forced'], result) + self.assertEqual( + result['exitcode'], + ApplicationRunner.ExitCode.UnknownException.value, + result, + ) + self.assertIn('cleanup entered', result['events']) + + def testRepeatedShutdownNotificationsDoNotExtendDeadline(self): + process = AppMainProcess(_successfulApplication) + try: + with mock.patch.object( + AppMainProcessModule.time, 'monotonic', return_value=100 + ): + process.beginShutdown(23) + with mock.patch.object( + AppMainProcessModule.time, 'monotonic', return_value=200 + ): + process.beginShutdown(23) + self.assertEqual(process._shutdownDeadline.value, 110) + process.beginShutdown() + self.assertEqual(process._shutdownExitCode.value, 23) + finally: + process.close() + def _checkSpawnedFailure(self, stage, *, createQt=False, failCrashWrite=False): for exceptionType, expected in ( (AssertionError, ApplicationRunner.ExitCode.AssertionError), @@ -299,7 +357,9 @@ class AppMainProcessTest(unittest.TestCase): (RuntimeError('runtime'), ApplicationRunner.ExitCode.UnknownException), ): with self.subTest(exception=type(exception).__name__): - owner = SimpleNamespace(saveCrashLog=mock.Mock()) + owner = SimpleNamespace( + saveCrashLog=mock.Mock(), beginShutdown=mock.Mock() + ) with ( mock.patch.object(AppMainProcessModule, 'APP', return_value=None), @@ -322,7 +382,7 @@ class AppMainProcessTest(unittest.TestCase): def testExceptionHookUsesPartiallyConstructedApplicationWhenAvailable(self): """Request application-owned exit after construction has succeeded.""" application = SimpleNamespace(exit=mock.Mock()) - owner = SimpleNamespace(saveCrashLog=mock.Mock()) + owner = SimpleNamespace(saveCrashLog=mock.Mock(), beginShutdown=mock.Mock()) exception = RuntimeError('runtime') with ( @@ -346,7 +406,7 @@ class AppMainProcessTest(unittest.TestCase): def testSignalHandlerIsSafeBeforeAndAfterApplicationConstruction(self): """Handle the exact signal without dereferencing absent application state.""" - owner = SimpleNamespace(application=None) + owner = SimpleNamespace(application=None, beginShutdown=mock.Mock()) with self.assertRaises(SystemExit) as raised: AppMainProcess.handler(owner, signal.SIGTERM, None) @@ -407,6 +467,23 @@ class CommandLineDispatchTest(unittest.TestCase): """Import the module after the test environment is established.""" self.module = importlib.import_module('Furious.__main__') + def testIntentionalForcedExitDoesNotShowACrashDialog(self): + process = mock.Mock(exitcode=-9) + process.waitForExit.return_value = 0 + with mock.patch.object( + self.module, 'AppMainProcess', return_value=process + ), mock.patch.object( + self.module, 'AppQMessageBox' + ) as dialog, self.assertRaises( + SystemExit + ) as raised: + self.module.runAppMain() + self.assertEqual(raised.exception.code, 0) + process.start.assert_called_once_with() + process.waitForExit.assert_called_once_with() + process.join.assert_not_called() + dialog.assert_not_called() + def testClearAndNormalCommandsUseOnlyTheirSelectedPath(self): """Dispatch the built-in clear command without starting the GUI child.""" with ( diff --git a/tests/test_architecture_refactors.py b/tests/test_architecture_refactors.py index 9ffbbba8..0c5c8520 100644 --- a/tests/test_architecture_refactors.py +++ b/tests/test_architecture_refactors.py @@ -350,6 +350,8 @@ class ApplicationLifecycleTransactionTest(TestCase): _exitCode=ApplicationRunner.ExitCode.ExitSuccess.value, setExitingFlag=mock.Mock(), cleanup=mock.Mock(), + shutdownRequested=mock.Mock(), + _shutdownRetryTimer=mock.Mock(), ) application._completeExit = functools.partial( DesktopApplication._completeExit, application @@ -364,6 +366,7 @@ class ApplicationLifecycleTransactionTest(TestCase): application.cleanup.assert_not_called() self.assertEqual(application._exitCode, 23) + application.shutdownRequested.emit.assert_called_once_with(23) finalExit.assert_called_once_with(23)