Skip to content

The Listener interface is at the wrong level of abstraction #636

@njsmith

Description

@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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions