Skip to content

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Feb 22, 2022

@graingert graingert changed the title skip getaddrinfo thread if host is already resolved, using socket.AI_NUMERIC... bpo-46824: skip getaddrinfo thread if host is already resolved, using socket.AI_NUMERIC... Feb 22, 2022
@@ -856,6 +797,15 @@ async def getaddrinfo(self, host, port, *,
else:
getaddr_func = socket.getaddrinfo

try:
return getaddr_func(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@graingert graingert force-pushed the getaddrinfo-numeric-only branch 3 times, most recently from 3ff77cd to feffb88 Compare February 22, 2022 15:10
@graingert
Copy link
Contributor Author

test_create_connection_ipv6_scope is failing because asyncio.get_running_loop().sock_connect ignores the flowinfo and scope of hosts resolved via getaddrinfo:

FAIL: test_create_connection_ipv6_scope (test.test_asyncio.test_base_events.BaseEventLoopWithSelectorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/contextlib.py", line 155, in __exit__
    self.gen.throw(typ, value, traceback)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/unittest/mock.py", line 1345, in decoration_helper
    yield (args, keywargs)
    ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/unittest/mock.py", line 1359, in patched
    return func(*newargs, **newkeywargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_asyncio/test_base_events.py", line 1244, in test_create_connection_ipv6_scope
    sock.connect.assert_called_with(('fe80::1', 80, 0, 1))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/unittest/mock.py", line 923, in assert_called_with
    raise AssertionError(_error_message()) from cause
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: expected call not found.
Expected: connect(('fe80::1', 80, 0, 1))
Actual: connect(('fe80::1', 80, 0, 0))

eg what should asyncio.get_running_loop().sock_connect(sock, ("google.com", 80, 0, 1)) do? currently it discards the scope=1 and uses whatever scope DNS getaddrinfo returns.

@graingert graingert force-pushed the getaddrinfo-numeric-only branch from feffb88 to 5c37749 Compare February 22, 2022 15:19
@graingert graingert marked this pull request as ready for review February 22, 2022 15:41
@graingert graingert force-pushed the getaddrinfo-numeric-only branch from 5c37749 to 3a87238 Compare February 22, 2022 22:32
@graingert graingert changed the title bpo-46824: skip getaddrinfo thread if host is already resolved, using socket.AI_NUMERIC... GH-90980: skip getaddrinfo thread if host is already resolved, using socket.AI_NUMERIC... May 20, 2022
Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets hope comments on slow Mac back in 2015 are resolved. Otherwise a nice optimisation.

@graingert graingert changed the title GH-90980: skip getaddrinfo thread if host is already resolved, using socket.AI_NUMERIC... gh-90980: skip getaddrinfo thread if host is already resolved, using socket.AI_NUMERIC... Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants