[Python-Dev] SocketServer issues (original) (raw)
Guido van Rossum guido at python.org
Wed Mar 14 17:44:07 CET 2012
- Previous message: [Python-Dev] SocketServer issues
- Next message: [Python-Dev] PEP 8 misnaming
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
2012/3/13 Kristján Valur Jónsson <kristjan at ccpgames.com>:
I want to mention some issues I‘ve had with the socketserver module, and discuss if there‘s a way to make it nicer.
So, for a long time we were able to create magic stackless mixin classes for it, like ThreadingMixIn, and assuming we had the appropriate socket replacement library, be able to use it nicely using tasklets. Then, at some point, the runforever loop was changed to support timeout through the use of select.select() before every socket.accept() call. This was very awkward because the whole concept of select() really goes contrary to the approach of using microthreads, non-blocking IO and all that.
I'm surprised -- surely a non-blocking framework should have no problem implementing select(), especially if it's for one file descriptor?
The only reason for this select call, was to support timeout for the accept. And even for vanilla applications, it necessiates an extra kernel call for every accept, just for the timeout.
I think it's fine to change the code so that if poll_interval is explicitly set to None, the select() call is skipped. I don't think the default should change though. The select() call is normally needed to support the shutdown() feature, which is very useful. And also the overridable service_actions() method.
Oh, there's another select() call, in handle_request(), that should also be skipped if timeout is None.
At least, I think a select() with a timeout of None blocks forever or until the socket is ready or until it is interrupted; I think this can always be skipped, since the immediately following I/O call will block in exactly the same way. Unless the socket is set in non-blocking mode; we may have to have provisions to avoid breaking that situation too.
The way around this for me has to do local modifications to the SocketServer and just get rid of the select.
I hope the above suggestion is sufficient? It's the best we can do while maintaining backward compatibility. This class has a lot of different features, and is designed to be subclassed, so it's hard to make changes that don't break anything.
So, my first question is: Why not simply rely on the already built-in timeout support in the socket module? Setting the correct timeout value on the accepting socket, will achieve the same thing. Of course, one then has to reset the timeout value on the accepted socket, but this is minor.
I don't think it's the same thing at all. If you set a timeout on the socket, the accept() or recvfrom() call in get_request() will raise an exception if no request comes in within the timeout (default 0.5 sec); with the timeout implemented in serve_forever(), get_request() and its callers won't have to worry about the timeout exception.
Second: Of late the SocketServer has grown additional features and _attributes. In particular, it now has two event objects, shutdownrequest _and isshutdown.
Notice the double underscores. They make it impossible to subclass the SocketServer class to provide a different implementation of runforever(). Is there any good reason why these attributes have been made „private“ like this? Having just seen Raymond‘s talk on how to subclass right, this looks like the wrong way to use the double leading underscores.
Assuming you meant serve_forever(), I see no problem at all. If you override serve_forever() you also have to override shutdown(). That's all. They are marked private because they are involved in subtle invariants that are easily disturbed if users touch them.
I could live with making them single-underscore protected, only to be used by knowledgeable subclasses. But not with making then public attributes.
So, two things really:
The use of select.select in SocketServer makes it necessary to subclass it to write a new version of runforever() for those that wish to use a non-blocking IO library instead of socket. And the presence of these private attributes make it (theoretically) impossible to specialize runforever in a mix-in class.
Any thoughs? Is anyone interested in seeing how the timeouts can be done without using select.select()? And what do you think about removing the double underscores from there and thus making serveforever owerrideable?
Let's see a patch (based on my concerns above) and then we can talk again.
-- --Guido van Rossum (python.org/~guido)
- Previous message: [Python-Dev] SocketServer issues
- Next message: [Python-Dev] PEP 8 misnaming
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]