python / asyncio

asyncio historical repository
https://docs.python.org/3/library/asyncio.html
1.03k stars 178 forks source link

Make loop TCP APIs accept only TCP streaming sockets #453

Closed 1st1 closed 7 years ago

1st1 commented 7 years ago

Currently, loop.create_server, loop.create_connection, and loop.connect_accepted_socket accept any kind of socket, although they are documented (and used by asyncio users) as TCP API.

Allowing those methods to accept any kinds of sockets leads to hard to debug bugs. For instance, aiohttp gunicorn worker simply passes all sockets to create_server (including UNIX sockets). We even have a unittest in asyncio that does that: test_create_datagram_endpoint_sock passes a UDP socket to create_connection.

TCP, UDP and UNIX sockets are different from each other and have some subtle API differences. We shouldn't allow to blindly pass any of them to create_server and other TCP APIs.

1st1 commented 7 years ago

(OT: connect_accepted_socket seems undocumented.)

I've committed Jim's documentation patch: http://bugs.python.org/issue27392. I've also updated this PR to address your comments.

gvanrossum commented 7 years ago

Thanks, LGTM now!

1st1 commented 7 years ago

I've updated the patch with more unittests and more robust checks of socket.type. Will merge it later.

1st1 commented 7 years ago

Thanks for the review, Guido. I've implemented the same set of features in uvloop, and will watch closely if they cause any problems (I highly doubt that).

gvanrossum commented 7 years ago

Whee!

1st1 commented 7 years ago

I rolled back the restriction of passing AF_UNIX to create_connection and create_server. Turns out too many people rely on that, so I guess we can just support that (it's easy to fix uvloop to support this as well).