Add loop.connect_accepted_socket

[jimfulton]

BPO	27392
Nosy	@gvanrossum, @vstinner, @jimfulton, @1st1
Files	connect_accepted_socket-doc.patch

^{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 = 'https://github.com/1st1'
closed_at = <Date 2016-10-06.18:10:40.888>
created_at = <Date 2016-06-26.16:05:16.970>
labels = ['type-feature', 'expert-asyncio']
title = 'Add loop.connect_accepted_socket'
updated_at = <Date 2016-11-07.20:36:55.274>
user = 'https://github.com/jimfulton'

bugs.python.org fields:

activity = <Date 2016-11-07.20:36:55.274>
actor = 'yselivanov'
assignee = 'yselivanov'
closed = True
closed_date = <Date 2016-10-06.18:10:40.888>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2016-06-26.16:05:16.970>
creator = 'j1m'
dependencies = []
files = ['43718']
hgrepos = []
issue_num = 27392
keywords = ['patch']
message_count = 44.0
messages = ['269293', '269297', '269315', '269492', '269493', '269494', '269513', '269515', '269516', '269519', '269524', '269536', '269540', '269545', '269546', '269547', '270046', '270052', '270059', '270060', '270061', '270062', '270133', '270182', '270185', '270272', '270273', '270411', '270785', '270850', '272129', '272177', '272178', '272179', '272203', '272204', '272205', '272206', '272207', '272232', '272233', '272280', '278201', '280236']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'vstinner', 'j1m', 'python-dev', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27392'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

[jimfulton]

The event loop create_connection method can take a socket to create a connection on an existing socket, including sockets obtained via an external listener. If an SSL context is provided, however, it assumes it's creating a client connection, making it impossible to use it in a server.

I've recently ported ZEO to asyncio. My original implementation used a separate thread for each connection and a thread for listening for connections. I think there are cases where this makes a lot of sense. Blocky operations only affect one client and, IMO, using an asynchronous model can simplify networking code even when there's only one connection. Unfortunately, this didn't work on Linux with SSL due to issues with SSL and non-blocking sockets. (Oddly, it worked fine on Mac OS X.)

I've switched my code to use create_server, but this has led to stability problems. Beyond http://bugs.python.org/issue27386, I'm seeing a lot of test instability. I can't get through the ZEO tests without some test failing, although the tests pass when run individually. I suspect that this is due to a problem in my error handling, asyncio's error handling, or likely both.

Note that the ZEO test suite is pretty ruthless, doing whatever they can to break ZEO servers and clients.

I have a version of my multi-threaded code that monkey-patches loop instances to pass server_side=True to _make_ssl_transport. With that awful hack, I can use an external listener and tests usually run without errors. :)

I'd be more than happy to create a PR that adds this option (including test and docs). Please just give me the word. :)

[1st1]

On Jun 26, 2016, at 12:05 PM, Jim Fulton <report@bugs.python.org> wrote:

I've switched my code to use create_server, but this has led to stability problems.

BTW, did you try to run ZEO tests on uvloop? I'm just curious if stability is somehow related to asyncio design, or its just implementation quirks.

[gvanrossum]

I'm confused. create_connection() is meant for creating client connection,
so I don't think a server_side flag makes sense. (There are lower-level
internal APIs that do take a server_side flag, but create_connection() is
just one caller for these.)

If create_server() is buggy we should fix those bugs!

[jimfulton]

Tests are also unstable with uvloop. (Although uvloop doesn't have http://bugs.python.org/issue27386 at least.)

I was eventually able to wrestle the test into submission using asyncio.Server. I suspect that some of this had to do with issues closing connections at the end of tests. I made my close logic more paranoid and that *seemed* to help. Some of the instability was due to test bugs that were activated by the different timing characteristics of using asyncio.Server.

[jimfulton]

Guido, IMO, there's value in having the ability to accept connections independently of handling them.

It wasn't clear to me from reading the documentation that create_connection is only meant for making client connections, especially given that it can take an already connected socket. It works well for non-SSL server sockets and would work for SSL with a trivial change.

This issue was discussed on the tulip list or issue tracker a couple of years ago. (Sorry, I can't find the link ATM.) I think it was dismissed because no one felt like looking into it and because you asserted that it makes no sense to use an async library when handling each connection in a separate thread.

IMO, async loop is useful even for a single connection when reading and writing are not purely synchronous. In ZEO it's very useful that that the server can send data to a client independent of requests the client is making. (Technically, under the hood this often means that there are really 2 I/O channels, the client's TCP connection and whatever mechanism the loop uses to allow itself to be woken asynchronously as in call_soon_threadsafe.)

[jimfulton]

I agree that if create_server (or asyncio.Server) is buggy, it should be fixed!

However, IMO, it's useful to be able to accept a connection outside of an asyncio event loop and then hand the loop the connected socket.

[1st1]

However, IMO, it's useful to be able to accept a connection outside of an asyncio event loop and then hand the loop the connected socket.

Looks like what you're asking for is a way to wrap existing socket object into a (transport, protocol) pair. I'm -1 to add this new semantics to loop.create_connection, as I think it will complicate it too much.

However, we can consider adding something like

loop.wrap_socket(protocol_factory, sock) -> Transport

[jimfulton]

Yury, I'm curious what you think the socket argument to create_connection is about.

[jimfulton]

BTW, a problem with this proposal that I realized after submitting it is that it changes an API that has multiple implementations, including implementations outside of the Python codebase. Arguably, this would require a PEP, at which point the change is no-longer trivial. :)

[1st1]

Yury, I'm curious what you think the socket argument to create_connection is about.

:) The current intended purpose of create_connection is to create a client connection. You're proposing to add a new argument -- server_side -- which I think will confuse the users of create_connection.

What I'm saying is that we may consider creating a low-level loop.wrap_socket, which would be generic and suitable to be used for both client and server connections. We could even refactor create_connection to use wrap_socket when 'sock' argument is passed to it.

We already have something similar, although it's a private API -- _make_socket_transport.

BTW, a problem with this proposal that I realized after submitting it is that it changes an API that has multiple implementations, including implementations outside of the Python codebase. Arguably, this would require a PEP, at which point the change is no-longer trivial. :)

No need for a PEP; Guido's approval is enough usually.

[jimfulton]

> Yury, I'm curious what you think the socket argument to create_connection is about.

:) The current intended purpose of create_connection is to create a client connection. You're proposing to add a new argument -- server_side -- which I think will confuse the users of create_connection.

Perhaps. I'll note that the word "client" appears nowhere in the documentation of create_connection. I needed a way to wrap a socket and create_connection took one. Wrapping a server socket seemed to be to be the most likely use case for it. <shrug>

What I'm saying is that we may consider creating a low-level loop.wrap_socket, which would be generic and suitable to be used for both client and server connections. We could even refactor create_connection to use wrap_socket when 'sock' argument is passed to it.

We already have something similar, although it's a private API -- _make_socket_transport.

Right. That's what I'm monkey-patching now to work around this, mostly as an experiment.

> BTW, a problem with this proposal that I realized after submitting it is that it changes an API that has multiple implementations, including implementations outside of the Python codebase. Arguably, this would require a PEP, at which point the change is no-longer trivial. :)

No need for a PEP; Guido's approval is enough usually.

/me holds breath

[gvanrossum]

Hm. The docs in PEP-3156 do mention that create_connection() is for clients (though it weakens this with "typically"): https://www.python.org/dev/peps/pep-3156/#internet-connections

I always think of TCP connections (which is what create_connection() is about) as essentially asymmetrical -- the server uses bind() and listen() and then sits in an accept() loop accepting connections, while the client reaches out to the server using connect(). And create_connection() is a wrapper around that connect() call; for servers we have create_server(). (The asymmetry in naming reflects the asymmetry in functionality and API.)

You can pass a pre-connected socket in to create_connection() for edge cases where you need control over how the socket is created and initialized or when there's a 3rd party library that can give you a connected socket that you want/need to use. Similarly, you can pass a pre-bound socket to create_server().

I guess at the lower, TCP level, there isn't much of a difference between a client-side socket and the kind of server-side socket that's returned by accept(). And maybe that's also the case for SSL (I've never actually understood most of the details of using SSL).

Maybe it's just culture shock? Or maybe we just need a public API that roughly represents the pair of calls to _make_ssl_transport() and _make_socket_transport() that are currently appearing both in _create_connection_transport() and in _accept_connection2(), plus some of the code around it that's a little tricky?

[jimfulton]

With SSL, the protocol is a little different clients and servers, although that may just be in the handshake. I'm no SSL expert by any means. When you call wrap_socket on an SSLContext, you can pass server_side, which defaults to False. If you get this wrong, you end up with an SSL protocol error.

FWIW, here's my awful monkey patch to work around this experimentally in ZEO:

zopefoundation/ZEO@daca97c#diff-248404a51b1503a38c3319c85e6c1c5aR174