python-trio / python-trio/trio

trio.socket.getaddrinfo may invoke a worker thread when it doesn't need to

Open
#1,249 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
7.3k
Forks
431
Avg merge
2d 17h
Merged PRs (30d)
6

Description

trio.socket.getaddrinfo tries to avoid invoking a worker thread when it knows that stdlib getaddrinfo won't block: the host is already an IP address and the port is already numeric. It does this by calling stdlib getaddrinfo with AI_NUMERICHOST|AI_NUMERICSERV added to the flags. If that call succeeds, it returns immediately, otherwise it calls getaddrinfo in a worker thread.

The problem is that trio assumes that a failure of this initial call is because we need to do a host and/or service lookup. This is not necessarily true. The most important case is when the caller of trio.socket.getaddrinfo already set AI_NUMERICHOST|AI_NUMERICSERV in the flags; in that case the call in the worker thread is going to fail in the exact same way that the call in the main thread failed, and there's no point going through the thread. Failures for unrelated reasons (e.g. inappropriate address family) will also fail the same way in the worker thread Failures are only retried in the worker thread if the getaddrinfo error code is EAI_NONAME, so we only need to worry about the cases where that can happen, but there are still several types of invalid "host" and/or "port" argument that will produce EAI_NONAME and there's no point retrying, even if caller didn't set AI_NUMERICHOST|AI_NUMERICSERV.

To fix this, getaddrinfo just needs to pay a little more attention to why the initial call failed and what the caller's flags were, and not retry calls that will provably fail the same way in the worker. This will also let us tell the authors of custom HostResolver implementations that they will not be called when both the host and the service were already numeric (but it's still possible for them to get an IP address with a service keyword, or a domain name with a numeric port) and they can rely on the rest of the arguments to have been validated already -- in particular they do not need to worry about the address family being something other than AF_INET, AF_INET6, or AF_UNSPEC.

I plan to work on this myself but I am filing this issue in advance to get advice about testing. Is there a good way to test "this operation does not do anything in a worker thread" already? I suppose I could define a test HostResolver that bombs out if called, since the logic is "call custom HR or else call stdlib getaddrinfo in a worker" if we get to that point, but maybe someone has a better idea?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at trio.socket.getaddrinfo and trace the initial stdlib call, its flags, and the retry path through HostResolver. Review existing resolver tests, then add coverage showing that provably invalid or already-numeric arguments do not invoke a worker or custom resolver, while valid lookup cases still do.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.