Skip to content

Commit 2e51f40

Browse files
[PR #10923/19643c9c backport][3.12] Fix weakref garbage collection race condition in DNS resolver manager (#10924)
Co-authored-by: J. Nick Koston <[email protected]>
1 parent 383323d commit 2e51f40

File tree

3 files changed

+51
-1
lines changed

3 files changed

+51
-1
lines changed

CHANGES/10923.feature.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
10847.feature.rst

aiohttp/resolver.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,14 @@ def release_resolver(
257257
loop: The event loop the resolver was using.
258258
"""
259259
# Remove client from its loop's tracking
260+
if loop not in self._loop_data:
261+
return
260262
resolver, client_set = self._loop_data[loop]
261263
client_set.discard(client)
262264
# If no more clients for this loop, cancel and remove its resolver
263265
if not client_set:
264-
resolver.cancel()
266+
if resolver is not None:
267+
resolver.cancel()
265268
del self._loop_data[loop]
266269

267270

tests/test_resolver.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,3 +628,49 @@ async def test_dns_resolver_manager_multiple_event_loops(
628628
# Verify resolver cleanup
629629
resolver1.cancel.assert_called_once()
630630
resolver2.cancel.assert_called_once()
631+
632+
633+
@pytest.mark.skipif(not getaddrinfo, reason="aiodns >=3.2.0 required")
634+
async def test_dns_resolver_manager_weakref_garbage_collection() -> None:
635+
"""Test that release_resolver handles None resolver due to weakref garbage collection."""
636+
manager = _DNSResolverManager()
637+
638+
# Create a mock resolver that will be None when accessed
639+
mock_resolver = Mock()
640+
mock_resolver.cancel = Mock()
641+
642+
with patch("aiodns.DNSResolver", return_value=mock_resolver):
643+
# Create an AsyncResolver to get a resolver from the manager
644+
resolver = AsyncResolver()
645+
loop = asyncio.get_running_loop()
646+
647+
# Manually corrupt the data to simulate garbage collection
648+
# by setting the resolver to None
649+
manager._loop_data[loop] = (None, manager._loop_data[loop][1]) # type: ignore[assignment]
650+
651+
# This should not raise an AttributeError: 'NoneType' object has no attribute 'cancel'
652+
await resolver.close()
653+
654+
# Verify no exception was raised and the loop data was cleaned up properly
655+
# Since we set resolver to None and there was one client, the entry should be removed
656+
assert loop not in manager._loop_data
657+
658+
659+
@pytest.mark.skipif(not getaddrinfo, reason="aiodns >=3.2.0 required")
660+
async def test_dns_resolver_manager_missing_loop_data() -> None:
661+
"""Test that release_resolver handles missing loop data gracefully."""
662+
manager = _DNSResolverManager()
663+
664+
with patch("aiodns.DNSResolver"):
665+
# Create an AsyncResolver
666+
resolver = AsyncResolver()
667+
loop = asyncio.get_running_loop()
668+
669+
# Manually remove the loop data to simulate race condition
670+
manager._loop_data.clear()
671+
672+
# This should not raise a KeyError
673+
await resolver.close()
674+
675+
# Verify no exception was raised
676+
assert loop not in manager._loop_data

0 commit comments

Comments
 (0)