This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: sockmodule.c: sock_connect vs negative errno values...
Type: behavior Stage:
Components: Extension Modules Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: icculus
Priority: normal Keywords:

Created on 2020-05-14 18:55 by icculus, last changed 2022-04-11 14:59 by admin.

Messages (1)
msg368863 - (view) Author: Ryan C. Gordon (icculus) Date: 2020-05-14 18:55
(Forgive any obvious mistakes in this report, I'm almost illiterate with Python, doubly so with Python internals.)

In trying to get buildbot-worker running on Haiku ( https://haiku-os.org/ ), it runs into a situation where it tries to connect a non-blocking TCP socket, which correctly reports EINPROGRESS, and cpython/Modules/sockmodule.c's internal_connect() returns this error code to sock_connect() and sock_connect_ex().

Both of the sock_connect* functions will return NULL if the error code is negative, but on Haiku, all the errno values are negative (EINPROGRESS, for example, is -2147454940).

I _think_ what sock_connect is intending to do here...

    res = internal_connect(s, SAS2SA(&addrbuf), addrlen, 1);
    if (res < 0)
        return NULL;

...is say "if we had a devastating and unexpected system error, give up immediately." Buildbot-worker seems to confirm this by throwing this exception in response:

  builtins.SystemError: <method 'connect_ex' of '_socket.socket' objects> returned NULL without setting an error

internal_connect returns -1 in those devastating-and-unexpected cases--namely when an exception is to be raised--and does not ever use that to otherwise signify a legit socket error. Linux and other systems don't otherwise fall into this "res < 0" condition because errno values are positive on those systems.

So I believe the correct fix here, in sock_connect() and sock_connect_ex(), is to check "if (res == -1)" instead of "res < 0" and let all other negative error codes carry on.

If this seems like the correct approach, I can assemble a pull request, but I don't know the full ramifications of this small change, so I thought I'd report it here first.

--ryan.
History
Date User Action Args
2022-04-11 14:59:31adminsetgithub: 84808
2020-05-14 18:55:17icculuscreate