The Listener interface is at the wrong level of abstraction

[njsmith]

We want to have an abstraction over the idea of "listening for incoming connection", because we want to be able to mix-and-match between protocol implementations and transport implementations – e.g. if you have an HTTP server or a Websocket server, you should be able to run it over TCP, over Tor, over TLS, over the haproxy PROXY protocol, over SSH tunneling, ... etc. Twisted gets a lot of mileage about of this idea with their "endpoints" system – one example that's particularly slick is txacme, which lets you transparently enable Let's Encrypt certs for any Twisted server. It works by defining a custom listener that detects when Let's Encrypt is connecting to validate a cert, and handles those connections automatically instead of passing them on to the application.

Our current abstraction is: a Listener is an object with accept and aclose methods. In practice, users mostly don't call these methods directly; the main user of this interface is serve_listeners, which calls accept in a loop, spawns tasks for incoming connections, and acloses the listener when cancelled.

This basically works, but over time I've been building up a list of little issues that make me itch, and wonder if maybe we've drawn the abstraction boundary at the wrong place.

Graceful shutdown of Unix-domain socket servers

If you have a server that's listening on a Unix socket, it's possible to do a zero-downtime upgrade, without any complicated protocol for handing off the socket between the old process and the new one. You can have the new process atomically bind to the listening address, and after that point all new incoming connections go to the new process, while existing connections stay with the old process. Then you tell the old process to shut down once all current connections are handled. (Haproxy for example does zero-downtime upgrades this way.)

To do this correctly, though, your accept loop needs a way to "drain" all the connections that have already happened, without waiting for any more to arrive. But that's not possible with the current Listener interface: we would need something like accept_nowait, which doesn't currently exist.

Maybe this doesn't matter because it's better to do zero-downtime upgrades via file descriptor passing, which is much more general solution (in particular, it can work for TCP services, which this can't!). Or we could add accept_nowait, which would work but is kind of awkward since then wrappers like SSLListener would have to wrap both accept and accept_nowait.

Another case where accept_nowait might be useful is in "batched accept" (see #14). Again, I'm not totally sure whether this is something we actually need, but maybe. Or batched accept might be something we need temporarily to work around other issues, but then eventually get rid of.

So... maybe accept alone isn't quite enough, but it isn't obvious what to do.

See also: #14 (discussion of accept_nowait), #147 (about how to request a graceful shutdown), #279 (general discussion of unix domain servers and how they can support zero-downtime upgrades)

Magic Let's Encrypt support

So let's say we want to implement something like txacme, and automatically handle incoming Let's Encrypt cert renewal. With the current system, this is possible, but has a few awkward features:

• We need a background task to manage the cert renewal scheduling. So you have to set that up somewhere, and then get a Listener object that assumes it's running.

• When a cert renewal is triggered, we need to know that someone is actually going to call accept soon. In practice this is true because everyone uses serve_listeners, but it's not actually guaranteed by the Listener interface.

• For each incoming connection that might be from Let's Encrypt, we need to go through the TLS handshake before we actually know whether it is Let's Encrypt. We can't do that directly in accept, because that will block other incoming connections. So, who's going to do it?

  • We could return the stream directly, and hope that the user will call send_all or receive_some soon, and then in there detect Let's Encrypt connections, handle the authentication, and then pretend to the user that this was some connection that just got dropped immediately. That would probably work OK in practice, but smells funny, because again it's making assumptions about how the user is calling accept.

  • Whenever we accept new connection, we could run a background task (in the same nursery as the cert renewal background task, I guess), that does the handshake, and then either handles the connection directly or else puts it in a queue or something to be returned by accept. Pretty awkward, and also has to assume that Listener.accept is being called in a loop (because each time accept is called, we have to optimistically accept lots of connections, and return some of them later).

Small warts of the current interface

Right now, serve_listeners has some special case code to handle EMFILE/ENFILE errors. These are a thing that can happen on Unix when SocketListener calls accept, and the proper way to handle them is to pause the accept loop for a bit. Theoretically, serve_listeners isn't supposed to know anything about SocketListener or Unix, but here it kind of has to, at least right now.

Another small wart is that most of the time, the thing you pass around to represent what a server might listen to is not actually a Listener, but rather a list of Listeners. For example, open_tcp_listeners returns a list, and serve_listeners takes a list. Not a huge deal, but a bit surprising. Why is it like this? Well, open_tcp_listeners might have multiple sockets, and you can't easily implement a single Listener that handles multiple sockets. A hypothetical MultiListener.accept would have to open a nursery and call accept on all of its sub-Listeners... and then what happens if several of them succeed at once?

A better abstraction?

So none of these is a smoking gun, but together they make me wonder if maybe we're approaching this wrong. In practice, everyone calls accept in a loop, and a lot of these issues seem to come from the fact that Listener implementations have to pretend that they don't know that.

Can we tweak the Listener interface so that it does know that?

One possibility I considered is: replace Listener.accept with Listener.accept_loop(send_channel) that simply accepts connections over and over, and calls send_channel.put on them. But... this would make wrapper classes like SSLListener pretty awkward, because they'd have to pull things off the channel, wrap them, and put them onto a new channel. You could make a WrapperListenerMixin that encapsulates the logic for doing this, but then we're back to inheritance rather than composition, which is a big warning sign for me – IMO one of the main advantages of Trio streams over Twisted protocols is that Trio streams work well with composition, while the inversion of control in Twisted protocols kinda forces the use of inheritance, and the accept_many approach is basically replicating Twisted's inverted-control approach.

Maybe it should just be... Listener.serve(nursery, handler)? This would basically mean moving serve_listeners into SocketListener (or SocketsListener?). So if we have a lot of primitive Listener types, there'd be potential for code duplication. But... not sure there really is much to listen to other than sockets. Wrapper classes would work by wrapping handler and then calling sublistener.serve.

[njsmith]

The main blocker here is that I think we also want to switch the handler's call signature to take the new connection as a kwarg, so that we can forward positional args without ambiguity (see #563). And that's fine, but... what should we call the kwarg?

My first idea was stream, but I don't like that. I'm thinking we'll make Listener[T] take a handle(*, something: T), and T might be something like WebsocketChannel, not just a Stream. So it needs to be something more neutral.

• peer
• connection
• conn
• client (I think this is my favorite so far)
• ...?

[njsmith]

The current trio docs use stream in examples. trio-websocket and hypercorn also use stream, possibly because they copied the docs.

The curio docs use client, which is where I got it.

The asyncio docs aren't helpful; their equivalents say (reader, writer).

[parity3]

Wouldn't callers of _serve_one_listener want to put some kind of backpressure (probably a semaphore acquire, with a release once the accepted connection closes) in there to accept() BEFORE you start running into os process limits? ie "I want to accept a max of 200 concurrent connections. Anything beyond should be subject to the backlog and then dropped or at the discretion of the os"?

This is mainly in the interest of general guidelines of backpressure -- never ask to do more until you know you have the resources to handle more. In a sync-world threadpool situation you'd be adding new connections to a bounded queue but here we're just calling nursery.start_soon.. at the very least a memory channel or some other callback should be used at the once-accepted stage, but ideally the wait should be just before the accept gets called in the first place, IMO.

[energizah]

It may be worth noting a few of the endpoint types that Twisted provides built-in.

Twisted provides guidelines to authors of libraries and applications on making endpoints configurable. One especially nice idea is defining a standard structure for endpoint into so it can be provided in a user config file, in envvars, on CLI, or over the network. Twisted's DSL is quite terse (e.g. tcp:host=twistedmatrix.com:port=80:timeout=15); I imagine an expanded format could be specified by jsonschema or similar.

[njsmith]

Reading this post about QUIC implementation made me realize that this is another case where the Listener.serve approach is better than Listener.accept. For QUIC, you have to do a handshake on incoming connections, and this takes place in userspace. In the post, the original implementation did this handshake inside the equivalent of accept, which meant that only one handshake could happen at a time. In the revised version, they switched to the equivalent of serve, and made it spawn a new task for each handshake, and then once the handshake was complete they re-use that task for running the connection handler.

I guess another option would be to immediately spawn the connection handler whenever a new handshake is started, and then do the handshake lazily the first time the connection handler tries to use the new stream. (This is how we handle TLS handshakes currently.) So I'm not sure this is a slam dunk argument by itself. But still, it's neat to see someone else comparing the exact same design approaches and coming to the same conclusion, in a totally different context.

@parity3

Wouldn't callers of _serve_one_listener want to put some kind of backpressure (probably a semaphore acquire, with a release once the accepted connection closes) in there to accept() BEFORE you start running into os process limits? ie "I want to accept a max of 200 concurrent connections. Anything beyond should be subject to the backlog and then dropped or at the discretion of the os"?

Unfortunately, backpressure doesn't really work for listeners. The point of a listener is that you're accepting connections from a bunch of diverse sources, so in general, even if you reject one incoming connection then that doesn't stop everyone else from continuing to send you new incoming connections at the same rate as before. And for TCP in particular, the OS backlog limit is really unhelpful; if it overflows, then the client doesn't get any notification, so their TCP stack treats it like a lost packet and will automatically block until some timeout expires and then automatically retry. So now your clients are getting lousy service, while you have the same amount of load, and you can't even pick and choose which clients you're serving. Generally the best option when overloaded is to continue to accept connections as fast as possible, then immediately close the ones you don't want. (Or in more extreme cases, use some external DoS-protection system to gatekeep the traffic and protect your squishy Python program. Serious DoS protection requires specialized tools.)

That said, with a Listener.serve style of interface it is totally possible to have a listener that uses some kind of policy to rate-limit the creation of new tasks. I think it's a relatively specialized feature so Trio's built-in listeners might not support it, but the point of having an abstract Listener interface is to make it easy to plug in new features like this after the fact.

@energizah

Twisted provides guidelines to authors of libraries and applications on making endpoints configurable. One especially nice idea is defining a standard structure for endpoint into so it can be provided in a user config file, in envvars, on CLI, or over the network. Twisted's DSL is quite terse (e.g. tcp:host=twistedmatrix.com:port=80:timeout=15); I imagine an expanded format could be specified by jsonschema or similar.

Yeah, Twisted's endpoint system is a very strong inspiration for Trio's stream/listener interfaces. The DSL part adds a lot of extra moving parts – not just a DSL parser, but also a whole plugin system, so that third-party libraries can extend the DSL – so I don't think it's a good fit for the Trio core. But definitely one of the goals here is that it should be possible to build a DSL like that on top of our stream/listener interfaces.