Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug that lead to infinite wait for dns futures #10529

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES/10529.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed an issue where dns queries were delayed indefinitely when an exception occurred in a ``tracer.send_dns_cache_miss``
-- by :user:`logioniz`.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Alexandru Mihai
Alexey Firsov
Alexey Nikitin
Alexey Popravka
Alexey Stavrov
Alexey Stepanov
Almaz Salakhov
Amin Etesamian
Expand Down
7 changes: 4 additions & 3 deletions aiohttp/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -1018,10 +1018,11 @@ async def _resolve_host_with_throttle(
This method must be run in a task and shielded from cancellation
to avoid cancelling the underlying lookup.
"""
if traces:
for trace in traces:
await trace.send_dns_cache_miss(host)
try:
if traces:
for trace in traces:
await trace.send_dns_cache_miss(host)

if traces:
for trace in traces:
await trace.send_dns_resolvehost_start(host)
Expand Down
55 changes: 55 additions & 0 deletions tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -3683,6 +3683,61 @@ async def send_dns_cache_hit(self, *args: object, **kwargs: object) -> None:
await connector.close()


async def test_connector_resolve_in_case_of_trace_cache_miss_exception(
loop: asyncio.AbstractEventLoop,
) -> None:
token: ResolveResult = {
"hostname": "localhost",
"host": "127.0.0.1",
"port": 80,
"family": socket.AF_INET,
"proto": 0,
"flags": socket.AI_NUMERICHOST,
}

request_count = 0

class DummyTracer(Trace):
def __init__(self) -> None:
"""Dummy"""

async def send_dns_cache_hit(self, *args: object, **kwargs: object) -> None:
"""Dummy send_dns_cache_hit"""

async def send_dns_resolvehost_start(
self, *args: object, **kwargs: object
) -> None:
"""Dummy send_dns_resolvehost_start"""

async def send_dns_resolvehost_end(
self, *args: object, **kwargs: object
) -> None:
"""Dummy send_dns_resolvehost_end"""

async def send_dns_cache_miss(self, *args: object, **kwargs: object) -> None:
nonlocal request_count
request_count += 1
if request_count <= 1:
raise Exception("first attempt")

async def resolve_response() -> List[ResolveResult]:
await asyncio.sleep(0)
return [token]

with mock.patch("aiohttp.connector.DefaultResolver") as m_resolver:
m_resolver().resolve.return_value = resolve_response()

connector = TCPConnector()
traces = [DummyTracer()]

with pytest.raises(Exception):
await connector._resolve_host("", 0, traces)

await connector._resolve_host("", 0, traces) == [token]

await connector.close()


async def test_connector_does_not_remove_needed_waiters(
loop: asyncio.AbstractEventLoop, key: ConnectionKey
) -> None:
Expand Down
Loading