classification
Title: add server.shutdown() method to SocketServer
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder: Uses of SocketServer.BaseServer.shutdown have a race
View: 2302
Assigned To: jyasskin Nosy List: akuchling, jyasskin, phr, pilcrow, skip.montanaro, werneck, zanella
Priority: normal Keywords: patch

Created on 2005-05-02 03:04 by phr, last changed 2008-03-16 17:20 by jyasskin. This issue is now closed.

Files
File name Uploaded Description Edit
issue1193577.patch werneck, 2008-02-25 19:26 diff from current trunk, SocketServer and test_socketserver
polling_shutdown.patch jyasskin, 2008-03-01 21:44 werneck's patch, merged up to r61161
polling_shutdown.patch jyasskin, 2008-03-02 08:18 timeouts reorganized; shutdown blocks
polling_shutdown.patch jyasskin, 2008-03-02 20:23 +documentation
Messages (9)
msg54518 - (view) Author: paul rubin (phr) Date: 2005-05-02 03:04
First of all the daemon_threads property on socket
servers should be added as an optional arg to the
server constructor, so you can say:
     TCPServer(..., daemon_threads=True).serve_forever()

instead of

    temp = TCPServer(...)
    temp.daemon_threads=True
    temp.serve_forever

Secondly there should be a way to STOP the server, once
you've started serving forever!  A request handler
should be able to say

   self.server.stop_serving()

This would work by setting a flag in the server object.
 The serve_forever loop would use select with a timeout
(default 0.5 sec, also settable as an optional arg to
the server constructor) or alternatively set a timeout
on the socket object.  So it would block listening for
new connections, but every 0.5 sec it would check the
stop_serving flag and break out of the loop if the flag
was set.

This method was suggested by Skip Montanaro in a clpy
thread about how to stop SocketServer and it's useful
enough that I think it should be part of the standard
impl.  I can make a patch if it's not obvious how to do it.
msg62804 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-02-23 20:02
This is getting in my way, so I'll take a look at it. I'm planning to
model the shutdown API after
http://java.sun.com/javase/6/docs/api/java/util/concurrent/ExecutorService.html.
The serve_forever->shutdown interval should probably also be available
through a context manager.
msg62994 - (view) Author: Pedro Werneck (werneck) Date: 2008-02-25 19:26
I had some code here to do the exact same thing with XML-RPC server. The
patch adds the feature to SocketServer, with a server.shutdown() method
to stop serve_forever(), and change the unittest to use and test the
feature.

I disagree on adding the daemon_threads arg as a keyword to TCPServer
since that's a feature of ThreadingMixIn
msg63170 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-03-01 21:44
Polling isn't a great way to handle shutdown, since it wastes CPU time
and decreases responsiveness, but it's also easy and my attempt to avoid
it isn't getting anywhere, so I'm planning to fix up your patch a bit
and commit it. Thanks!

I've merged your patch with my conflicting change in the trunk and
re-attached it.

Two things:
1) This patch may interfere with the existing timeout in await_request.
We may be able to re-use that instead of having two select statements.

2) I believe it's important to provide a way to block until the server
isn't accepting any more requests and to block until all active requests
are finished running. If the active requests depend on other bits of the
system, blocking until they're done would help them terminate
gracefully. It would also be useful to give users a more proactive way
to kill active requests, perhaps by listing the handler objects
associated with them (or their PIDs for forking servers). It's
surprisingly complicated to avoid race conditions in these
implementations, especially without support from the Server object.
msg63172 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-03-01 21:55
Also, Pedro's argument against a daemon_threads argument to TCPServer
convinces me. I think we can add it in ThreadingMixIn.__init__ once this
whole hierarchy is switched over to new-style classes. That's already
done in 3.0, but I don't know what it would affect in 2.6. If anyone
wants to keep pushing it, would you open a new bug?
msg63177 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-03-02 08:18
It seems that .await_request() was only added a month ago to fix issue
742598, so it's no great hardship to refactor it again now. Timeouts
never worked for .serve_forever() because the try/except in
.handle_request() turned their exception into a plain return, which
didn't stop .server_forever() from looping. Timeouts also seem to be
unnecessary in a situation in which shutdown works. Shutdown can only be
called from a separate thread, and when you have more threads you can
also do periodic tasks in them.

So I've made that explicit: the .serve_forever/shutdown combination
doesn't handle timeouts. [This has nothing to do with the fact that it
takes three times as much code to handle them. Don't look at the excuse
behind the curtain. ;)]

This patch needs some more documentation and a NEWS entry before it gets
submitted, but I want to check that everyone would be happy with the
concept.
msg63185 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-03-02 20:23
I'll submit the attached patch in a couple days unless I get comments.
msg63345 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-03-07 06:27
Hearing no objections, I've submitted this as r61289.
msg63587 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2008-03-16 17:20
So this has a race. See issue 2302 to discuss a fix.
History
Date User Action Args
2008-03-16 17:20:30jyasskinsetstatus: open -> closed
resolution: fixed
superseder: Uses of SocketServer.BaseServer.shutdown have a race
messages: + msg63587
2008-03-07 06:27:46jyasskinsetmessages: + msg63345
2008-03-02 20:23:07jyasskinsetfiles: + polling_shutdown.patch
messages: + msg63185
2008-03-02 08:18:39jyasskinsetfiles: + polling_shutdown.patch
nosy: + akuchling, pilcrow
messages: + msg63177
versions: + Python 2.6, Python 3.0
2008-03-01 21:55:44jyasskinsetmessages: + msg63172
title: add server.shutdown() method and daemon arg to SocketServer -> add server.shutdown() method to SocketServer
2008-03-01 21:44:56jyasskinsetfiles: + polling_shutdown.patch
messages: + msg63170
2008-02-25 19:26:49wernecksetfiles: + issue1193577.patch
nosy: + werneck
messages: + msg62994
keywords: + patch
2008-02-25 18:17:16zanellasetnosy: + zanella
2008-02-23 20:02:59jyasskinsetassignee: skip.montanaro -> jyasskin
messages: + msg62804
nosy: + jyasskin
2005-05-02 03:04:36phrcreate