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,