2012/3/13 Kristján Valur Jónsson <krist...@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 run_forever 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, __shutdown_request > and __is_shut_down. > > Notice the double underscores. > > They make it impossible to subclass the SocketServer class to provide a > different implementation of run_forever(). 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 run_forever() 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 run_forever 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 serve_forever owerrideable? Let's see a patch (based on my concerns above) and then we can talk again. -- --Guido van Rossum (python.org/~guido) _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
