mirror of
https://github.com/LorenEteval/Furious.git
synced 2026-09-22 23:08:08 +03:00
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 <loren.eteval@proton.me>
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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 (
|
||||
|
||||
+14
-12
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user