Author vstinner
Recipients neologix, pitrou, pturing, python-dev, sbt, vstinner
Date 2014-07-23.23:34:33
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <CAMpsgwaf3Yv17ZhQxKyTjicPffp-aCsXBnZCKmGwx8YXshhYFw@mail.gmail.com>
In-reply-to <CAH_1eM1L_8Zba0kUyu_J8Jz=tQNnqFrKAH2-espndVzbUAknjw@mail.gmail.com>
Content
2014-07-24 0:21 GMT+02:00 Charles-Fran├žois Natali <report@bugs.python.org>:
>> You should copy the code from asyncio.windows_utils.socketpair(). It checks also type and proto parameter: raise a ValueError for unsupported values (only SOCK_STREAM and proto=0 are supported). It also supports IPv6. It handles BlockingIOError and InterruptedError on connect (I never understood why these exceptions are simply ignored).
>
> I could use it then, although all those checks are unnecessary since
> the underlying socket() call will check it for us.

socket.socket() supports SOCK_DGRAM on Windows, but asyncio
socketpair() function does not. The check on the socket family is just
to raise a better error message than an error on connect() or
something else.

I don't remember why I added a specific check on the proto parameter.

We might support more protocols in the future, but I'm not sure that
it's interesting to support them (right now). I  guess that most
people will call socketpair() without any parameter.

>> You patch checks the address received by accept() and retry. It's a little bit surprising. If you get an unexpected connection, maybe an exception should be raised, instead of just ignoring it. Maybe we should modify the code in asyncio to check also the address?
>
> Why?
> If you have an unexpected connection, it should be dropped, and we
> should wait until the client we're expecting connects, there's no
> reason to raise an error.
> But I'll just reuse asyncio's version.

Your version is safer. You should reuse your while dropping unknown connections.

By the way, we should reuse socket.socketpair() in
asyncio.windows_utils. The Tulip is written for Python 3.3 and shares
exactly the same code base, so you should write

Something like:

if hasattr(socket, 'socketpair'):
    socketpair = socket.socketpair
else:
  def socketpair(...): ...

Please also fix socketpair() in asyncio to add the while/drop unknown
connection (well, the function in socket.py and windows_utils.py must
be the same).

> Well, that's what I suggested initially, but never got any reply, so I
> went for the least intrusive. I personally think too it should belong
> to socket module.

On Windows, asyncio uses a socket pair in its default event loop
(SelectorEventLoop) for its "self pipe", because select.select() only
supports sockets. The Windows ProactorEventLoop also uses a socket
pair for its "self pipe".

Oh, and you forgot to modify the documentation to update
"Availability". Please add a ".. versionchanged:: 3.5" mentionning
that the function is now also available on Windows.
History
Date User Action Args
2014-07-23 23:34:34vstinnersetrecipients: + vstinner, pitrou, neologix, python-dev, sbt, pturing
2014-07-23 23:34:34vstinnerlinkissue18643 messages
2014-07-23 23:34:34vstinnercreate