From 1dc0d92c1a5c0b67e6e4c83fc46ec17716c3bd4d Mon Sep 17 00:00:00 2001 From: Loren Eteval Date: Mon, 24 Aug 2026 18:40:16 +0800 Subject: [PATCH] Scope DNS resolver to connection lifecycle Create and dispose the DNS resolver with its owning connection manager instead of at module import time. Make cleanup explicit and keep architecture and external-core tests aligned with injected resolver ownership. Signed-off-by: Loren Eteval --- Furious/Service/AGENTS.md | 2 ++ Furious/Service/ConnectionManager.py | 25 +++++++++++++++++++++++-- Furious/Service/DnsResolver.py | 11 +++++++++-- tests/test_architecture_refactors.py | 20 ++++++++++++++++++++ tests/test_external_core.py | 26 ++++++++++++++------------ 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/Furious/Service/AGENTS.md b/Furious/Service/AGENTS.md index f944bd3..c0bdc66 100644 --- a/Furious/Service/AGENTS.md +++ b/Furious/Service/AGENTS.md @@ -2,6 +2,8 @@ - Services own workflows and temporary resources; controllers own shared application state. Services may use Qt for asynchronous I/O/signals but must not own pages or encode presentation policy. +- Never instantiate `QObject` services such as network-access managers at module import time. Acquire them after the + application exists, give them one explicit service/application owner, and release them during that owner's cleanup. - Inject repositories, runtimes, clients, clocks, and callbacks where practical. One service owns each worker, reply, timer, executor, process, and cache; cleanup is bounded and idempotent. diff --git a/Furious/Service/ConnectionManager.py b/Furious/Service/ConnectionManager.py index fe22c87..01e6ac7 100644 --- a/Furious/Service/ConnectionManager.py +++ b/Furious/Service/ConnectionManager.py @@ -130,12 +130,24 @@ class ConnectionManager(Mixins.CleanupOnExit): def __init__(self, *args, **kwargs): """Initialize the connection manager.""" + self._dnsResolver = kwargs.pop('dnsResolver', None) + super().__init__(*args, **kwargs) self.uniqueCleanup = False self.runtimes = list() self._lastStartError = '' + def _connectionDnsResolver(self) -> DnsResolver: + """Return the resolver owned by this connection-manager lifecycle.""" + if self._dnsResolver is None: + # Creating QNetworkAccessManager during module import is invalid: + # QApplication does not exist yet. DNS is needed only for the + # application-managed TUN path, so acquire it at that boundary. + self._dnsResolver = DnsResolver() + + return self._dnsResolver + @property def lastStartError(self) -> str: """Return the concise failure reported by the latest runtime start.""" @@ -487,9 +499,10 @@ class ConnectionManager(Mixins.CleanupOnExit): address = configcopy.remoteAddress() if not isValidIPAddress(address): - DnsResolver.configureHttpProxy(configcopy.httpProxy()) + dnsResolver = self._connectionDnsResolver() + dnsResolver.configureHttpProxy(configcopy.httpProxy()) - error, resolved = DnsResolver.resolve(address) + error, resolved = dnsResolver.resolve(address) if error: SystemRoutingTable.managedRoutes.clear() @@ -742,3 +755,11 @@ class ConnectionManager(Mixins.CleanupOnExit): def cleanup(self): """Release resources owned by the core manager.""" self.stopAll() + + if self._dnsResolver is not None: + dispose = getattr(self._dnsResolver, 'dispose', None) + + if callable(dispose): + dispose() + + self._dnsResolver = None diff --git a/Furious/Service/DnsResolver.py b/Furious/Service/DnsResolver.py index d285d76..c6cc30b 100644 --- a/Furious/Service/DnsResolver.py +++ b/Furious/Service/DnsResolver.py @@ -35,7 +35,7 @@ __all__ = ['DnsResolver'] logger = logging.getLogger(__name__) -class _DnsResolver(HttpGetManager): +class DnsResolver(HttpGetManager): """Represent DNS resolver.""" MAX_REFERENCE_DEPTH = 32 @@ -258,5 +258,12 @@ class _DnsResolver(HttpGetManager): ): networkReply.abort() + def dispose(self): + """Abort pending replies and schedule this resolver for destruction.""" + for networkReply in tuple(self._replyContexts): + if not networkReply.isFinished(): + networkReply.abort() -DnsResolver = _DnsResolver() + self._replyContexts.clear() + + self.deleteLater() diff --git a/tests/test_architecture_refactors.py b/tests/test_architecture_refactors.py index c4c8b48..0806b0c 100644 --- a/tests/test_architecture_refactors.py +++ b/tests/test_architecture_refactors.py @@ -556,6 +556,26 @@ class ApplicationLifecycleTransactionTest(TestCase): class ConnectionStartupTransactionTest(TestCase): """Verify one failed attempt releases only its own exact runtimes.""" + def testDnsResolverIsAcquiredLazilyAndReleasedByTheManager(self): + """Avoid constructing a Qt network manager before QApplication exists.""" + resolver = mock.Mock() + manager = ConnectionManager() + + with mock.patch( + 'Furious.Service.ConnectionManager.DnsResolver', + return_value=resolver, + ) as resolverFactory: + resolverFactory.assert_not_called() + self.assertIs(manager._connectionDnsResolver(), resolver) + self.assertIs(manager._connectionDnsResolver(), resolver) + + resolverFactory.assert_called_once_with() + + manager.cleanup() + + resolver.dispose.assert_called_once_with() + self.assertIsNone(manager._dnsResolver) + @staticmethod def _start(manager, runtime, *, success): with ( diff --git a/tests/test_external_core.py b/tests/test_external_core.py index af49d81..f2863db 100644 --- a/tests/test_external_core.py +++ b/tests/test_external_core.py @@ -275,8 +275,10 @@ class ExternalCoreProcessTest(unittest.TestCase): registry = mock.Mock() registry.prepareTUN.return_value = False registry.usesApplicationTun2socks.return_value = True + dnsResolver = mock.Mock() + dnsResolver.resolve.return_value = (True, []) - manager = NoCoreRuntimeConnectionManager() + manager = NoCoreRuntimeConnectionManager(dnsResolver=dnsResolver) with ( mock.patch( @@ -315,19 +317,12 @@ class ExternalCoreProcessTest(unittest.TestCase): 'Furious.Service.ConnectionManager.SystemRoutingTable.delete' ), mock.patch('Furious.Service.ConnectionManager.Tun2socks'), - mock.patch( - 'Furious.Service.ConnectionManager.DnsResolver.configureHttpProxy' - ), - mock.patch( - 'Furious.Service.ConnectionManager.DnsResolver.resolve', - return_value=(True, []), - ) as resolve, ): self.assertFalse(manager.start(config, '', deepcopy=False)) - resolve.assert_called_once_with('actual-server.example.com') + dnsResolver.resolve.assert_called_once_with('actual-server.example.com') - self.assertNotEqual(resolve.call_args.args[0], executable) + self.assertNotEqual(dnsResolver.resolve.call_args.args[0], executable) manager.cleanup() @@ -479,6 +474,7 @@ class DnsResolverRobustnessTest(unittest.TestCase): } DnsResolver.successCallback( + mock.Mock(), Reply(), domain='missing.example', resultMap=result, @@ -515,8 +511,11 @@ class DnsResolverRobustnessTest(unittest.TestCase): 'visited': {'a.example', 'b.example'}, } - with mock.patch.object(DnsResolver, 'webGET') as webGet: + resolver = mock.Mock() + + with mock.patch.object(resolver, 'webGET') as webGet: DnsResolver.successCallback( + resolver, Reply(), domain='b.example', resultMap=result, @@ -556,8 +555,11 @@ class DnsResolverRobustnessTest(unittest.TestCase): 'visited': {'current.example'}, } - with mock.patch.object(DnsResolver, 'webGET') as webGet: + resolver = mock.Mock(MAX_REFERENCE_DEPTH=DnsResolver.MAX_REFERENCE_DEPTH) + + with mock.patch.object(resolver, 'webGET') as webGet: DnsResolver.successCallback( + resolver, Reply(), domain='current.example', resultMap=result,