STINNER Victor added the comment:

2014-07-24 0:21 GMT+02:00 Charles-François Natali <rep...@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.

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue18643>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to