sockmodule.c: sock_connect vs negative errno values...

[rcgordon]

BPO	40628
Nosy	@rcgordon

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-05-14.18:55:16.943>
labels = ['extension-modules', 'type-bug', '3.7']
title = 'sockmodule.c: sock_connect vs negative errno values...'
updated_at = <Date 2020-05-14.18:55:16.943>
user = 'https://github.com/rcgordon'

bugs.python.org fields:

activity = <Date 2020-05-14.18:55:16.943>
actor = 'icculus'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2020-05-14.18:55:16.943>
creator = 'icculus'
dependencies = []
files = []
hgrepos = []
issue_num = 40628
keywords = []
message_count = 1.0
messages = ['368863']
nosy_count = 1.0
nosy_names = ['icculus']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue40628'
versions = ['Python 3.7']

Linked PRs

• gh-84808: Handle errors returned by internal_connect() #122304

[rcgordon]

(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.

[iritkatriel]

Since Haiku is not a supported platform, I'm relabelling this from bug to feature request. I don't know how likely it is that it will be implemented though, there would need to be a core developer who cares enough about Haiku to do something non-standard for it (an assess what the implications of that may be).

[encukou]

IMO, Python should not assume that errno codes are positive. The point of PEP-11 is to not maintain speedups/workarounds for niche platforms, not to overfit CPython to rely on quirks of the tiered platforms.
To me, this is a legitimate bug, though one that core devs won't fix alone, and might not prioritize.

Please send the PR. Considering the ramifications of the change is the reviewer's job.

[erlend-aasland]

I agree with Petr; this definitely sounds like a bug, not a feature.