python / cpython

The Python programming language
https://www.python.org
Other
62.59k stars 30.04k forks source link

Timeout is doubled - socket.create_connection() #123793

Open j-schulz opened 3 weeks ago

j-schulz commented 3 weeks ago

Bug report

Bug description:

import socket
import time
start = time.time()
sock = socket.create_connection(
    address=("foobar", 10000),
    timeout=5)
print(time.time() - start)

No matter what value you specify as a timeout, the timeout is always doubled. If you specify 5 seconds, an exception will only be thrown after 10 seconds; if you specify 10 seconds, then only after 20.

Tested on Debian 12 with Python 3.11.

CPython versions tested on:

3.11

Operating systems tested on:

Linux

graingert commented 3 weeks ago

Probably your host has multiple IP addresses, eg IPv4 and IPv6, if you pass all_errors=True you'll see multiple TimeoutErrors

j-schulz commented 3 weeks ago

Yes, that's right, the host is resolved to 2 addresses. This of course means that you can no longer control how long create_connection should run in total, unless you build something around it yourself.

If a host is resolved to 10 IP addresses, for example, this leads to a timeout of 100 seconds. However, the ideal solution would be to have the option of setting a timeout that expires after the set time, regardless of how many IP addresses are resolved.

graingert commented 2 weeks ago

However, the ideal solution would be to have the option of setting a timeout that expires after the set time, regardless of how many IP addresses are resolved.

If that's the implementation it's likely that one IP that never responds will consume the entire timeout and no further IPs will be attempted. Happy eyeballs could be implemented using non blocking sockets and selectors module but that's likely too complicated

picnixz commented 2 weeks ago

Actually, we could ask @sethmlarson whether we could integrate his implementation: https://github.com/sethmlarson/rfc6555.

j-schulz commented 2 weeks ago

In my opinion, an option in the form of max_tries would be sufficient to gain some control over the maximum runtime.

picnixz commented 2 weeks ago

With a max_retry value, I'd suggest implementing exponential backoff if this is not already the case. OTOH, how do you suggest we use a timeout in this case?

j-schulz commented 2 weeks ago

Timeout remains per IP. With max_retry you can then specify the maximum number of IPs that should be tried. If you also add a logic that switches between v4 and v6 during the retries, I think it would be perfect. Based on this, I can assume that if I set a timeout of 10 seconds and max_retry to 3, the connection attempt will take a maximum of 30 (+overhead) seconds.

j-schulz commented 2 weeks ago

OT: There is a similar problem with wrap_socket in SSL. With extremely slow connections - or if someone is evil enough to only send 1 byte per second - the SSL handshake also hangs forever. You have no control over the maximum runtime of the handshake here either, unless you tinker around with it yourself.

picnixz commented 2 weeks ago

Then a fundamental question remains on which IPs to choose. The issue with a max_retry is that it depends on the output of getaddrinfo which I think is deterministic. One problem you could have is that the actual "good" address is at the end of the list being returned by getaddrinfo. I think trying the different addresses using Happy Eyeballs should be preferred in this case (but I don't have a strong opinion on the idea).

If you also add a logic that switches between v4 and v6 during the retries, I think it would be perfect.

That could resolve some issues with priorities but not all I think unfortunately :(

picnixz commented 2 weeks ago

OT: There is a similar problem with wrap_socket in SSL. With extremely slow connections - or if someone is evil enough to only send 1 byte per second - the SSL handshake also hangs forever. You have no control over the maximum runtime of the handshake here either, unless you tinker around with it yourself.

Could you file a separate issue for this one please? (you can link this issue as being related though).

j-schulz commented 2 weeks ago

Then a fundamental question remains on which IPs to choose.

I wouldn't go that far, because when you try to connect you don't know which IPs are reachable and which aren't. Fortunately, getaddrinfo returns an array with v4 and v6. If you sort these by v4 and v6 beforehand, you try one of the two versions alternately until max_retry is used up. This also eliminates the problem if the local host can't speak either v4 or v6, so at least with a max_retry of 2, each version is tested once.