Issue1193577
Created on 2005-05-02 03:04 by phr, last changed 2022-04-11 14:56 by admin. 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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
Date: 2008-03-07 06:27 | |
Hearing no objections, I've submitted this as r61289. |
|||
| msg63587 - (view) | Author: Jeffrey Yasskin (jyasskin) * ![]() |
Date: 2008-03-16 17:20 | |
So this has a race. See issue 2302 to discuss a fix. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:11 | admin | set | github: 41934 |
| 2008-03-16 17:20:30 | jyasskin | set | status: open -> closed resolution: fixed superseder: Uses of SocketServer.BaseServer.shutdown have a race messages: + msg63587 |
| 2008-03-07 06:27:46 | jyasskin | set | messages: + msg63345 |
| 2008-03-02 20:23:07 | jyasskin | set | files:
+ polling_shutdown.patch messages: + msg63185 |
| 2008-03-02 08:18:39 | jyasskin | set | files:
+ polling_shutdown.patch nosy: + akuchling, pilcrow messages: + msg63177 versions: + Python 2.6, Python 3.0 |
| 2008-03-01 21:55:44 | jyasskin | set | messages:
+ msg63172 title: add server.shutdown() method and daemon arg to SocketServer -> add server.shutdown() method to SocketServer |
| 2008-03-01 21:44:56 | jyasskin | set | files:
+ polling_shutdown.patch messages: + msg63170 |
| 2008-02-25 19:26:49 | werneck | set | files:
+ issue1193577.patch nosy: + werneck messages: + msg62994 keywords: + patch |
| 2008-02-25 18:17:16 | zanella | set | nosy: + zanella |
| 2008-02-23 20:02:59 | jyasskin | set | assignee: skip.montanaro -> jyasskin messages: + msg62804 nosy: + jyasskin |
| 2005-05-02 03:04:36 | phr | create | |
