From bdbd09733a7744094499d7509cc3ed6c69633287 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Thu, 12 Jun 2025 11:32:53 +0200 Subject: [PATCH] Avoid aiodns resolver memory leak (#5941) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Avoid aiodns resolver memory leak In certain cases, the aiodns resolver can leak memory. This also leads to Fatal `Python error… ffi.from_handle()`. This addresses the issue by ensuring that the resolver is properly closed when it is no longer needed. * Address coderabbitai feedback * Fix pytest * Fix pytest --- supervisor/resolution/checks/dns_server.py | 28 +++++++++++++------ .../resolution/checks/dns_server_ipv6.py | 15 ++++------ tests/resolution/check/test_check.py | 4 --- .../check/test_check_dns_server_ipv6_error.py | 2 +- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/supervisor/resolution/checks/dns_server.py b/supervisor/resolution/checks/dns_server.py index 55f377e72..602c00898 100644 --- a/supervisor/resolution/checks/dns_server.py +++ b/supervisor/resolution/checks/dns_server.py @@ -15,6 +15,24 @@ from ..const import DNS_CHECK_HOST, ContextType, IssueType from .base import CheckBase +async def check_server( + loop: asyncio.AbstractEventLoop, server: str, qtype: str +) -> None: + """Check a DNS server and report issues.""" + ip_addr = server[6:] if server.startswith("dns://") else server + resolver = DNSResolver(loop=loop, nameservers=[ip_addr]) + try: + await resolver.query(DNS_CHECK_HOST, qtype) + finally: + + def _delete_resolver(): + """Close resolver to avoid memory leaks.""" + nonlocal resolver + del resolver + + loop.call_later(1, _delete_resolver) + + def setup(coresys: CoreSys) -> CheckBase: """Check setup function.""" return CheckDNSServer(coresys) @@ -33,7 +51,7 @@ class CheckDNSServer(CheckBase): """Run check if not affected by issue.""" dns_servers = self.dns_servers results = await asyncio.gather( - *[self._check_server(server) for server in dns_servers], + *[check_server(self.sys_loop, server, "A") for server in dns_servers], return_exceptions=True, ) for i in (r for r in range(len(results)) if isinstance(results[r], DNSError)): @@ -51,18 +69,12 @@ class CheckDNSServer(CheckBase): return False try: - await self._check_server(reference) + await check_server(self.sys_loop, reference, "A") except DNSError: return True return False - async def _check_server(self, server: str): - """Check a DNS server and report issues.""" - ip_addr = server[6:] if server.startswith("dns://") else server - resolver = DNSResolver(nameservers=[ip_addr]) - await resolver.query(DNS_CHECK_HOST, "A") - @property def dns_servers(self) -> list[str]: """All user and system provided dns servers.""" diff --git a/supervisor/resolution/checks/dns_server_ipv6.py b/supervisor/resolution/checks/dns_server_ipv6.py index ab3d7bb1a..58e97ada5 100644 --- a/supervisor/resolution/checks/dns_server_ipv6.py +++ b/supervisor/resolution/checks/dns_server_ipv6.py @@ -3,15 +3,16 @@ import asyncio from datetime import timedelta -from aiodns import DNSResolver from aiodns.error import DNSError +from supervisor.resolution.checks.dns_server import check_server + from ...const import CoreState from ...coresys import CoreSys from ...jobs.const import JobCondition, JobExecutionLimit from ...jobs.decorator import Job from ...utils.sentry import async_capture_exception -from ..const import DNS_CHECK_HOST, DNS_ERROR_NO_DATA, ContextType, IssueType +from ..const import DNS_ERROR_NO_DATA, ContextType, IssueType from .base import CheckBase @@ -33,7 +34,7 @@ class CheckDNSServerIPv6(CheckBase): """Run check if not affected by issue.""" dns_servers = self.dns_servers results = await asyncio.gather( - *[self._check_server(server) for server in dns_servers], + *[check_server(self.sys_loop, server, "AAAA") for server in dns_servers], return_exceptions=True, ) for i in ( @@ -58,19 +59,13 @@ class CheckDNSServerIPv6(CheckBase): return False try: - await self._check_server(reference) + await check_server(self.sys_loop, reference, "AAAA") except DNSError as dns_error: if dns_error.args[0] != DNS_ERROR_NO_DATA: return True return False - async def _check_server(self, server: str): - """Check a DNS server and report issues.""" - ip_addr = server[6:] if server.startswith("dns://") else server - resolver = DNSResolver(nameservers=[ip_addr]) - await resolver.query(DNS_CHECK_HOST, "AAAA") - @property def dns_servers(self) -> list[str]: """All user and system provided dns servers.""" diff --git a/tests/resolution/check/test_check.py b/tests/resolution/check/test_check.py index 4c8e5d7c3..a78a290eb 100644 --- a/tests/resolution/check/test_check.py +++ b/tests/resolution/check/test_check.py @@ -21,10 +21,6 @@ def fixture_mock_dns_query(): "supervisor.resolution.checks.dns_server.DNSResolver.query", new_callable=AsyncMock, ), - patch( - "supervisor.resolution.checks.dns_server_ipv6.DNSResolver.query", - new_callable=AsyncMock, - ), ): yield diff --git a/tests/resolution/check/test_check_dns_server_ipv6_error.py b/tests/resolution/check/test_check_dns_server_ipv6_error.py index 501f1a2d4..3eefc106a 100644 --- a/tests/resolution/check/test_check_dns_server_ipv6_error.py +++ b/tests/resolution/check/test_check_dns_server_ipv6_error.py @@ -15,7 +15,7 @@ from supervisor.resolution.const import ContextType, IssueType async def fixture_dns_query() -> AsyncMock: """Mock aiodns query.""" with patch( - "supervisor.resolution.checks.dns_server_ipv6.DNSResolver.query", + "supervisor.resolution.checks.dns_server.DNSResolver.query", new_callable=AsyncMock, ) as dns_query: yield dns_query