guido at python
Mar 14, 2012, 9:44 AM
Post #7 of 14
(228 views)
Permalink
|
2012/3/13 Kristján Valur Jónsson <kristjan [at] ccpgames>:
> 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 [at] python
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: http://mail.python.org/mailman/options/python-dev/list-python-dev%40lists.gossamer-threads.com
|
