This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: socketserver.ForkingMixin collect_children routine needs to collect only it's children
Type: resource usage Stage: resolved
Components: Versions: Python 3.3
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: giampaolo.rodola, neologix, orsenthil, pitrou
Priority: normal Keywords: patch

Created on 2011-05-26 05:19 by orsenthil, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
ss_wait_group.diff neologix, 2011-05-26 12:46 patch using process group review
Messages (4)
msg136931 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-05-26 05:19
socketserver.ForkingMixin class has a collect_children method, that waits for the children to exit.
But, that waits for any child process and not just the ones spawned the socketserver alone.

            try:
                pid, status = os.waitpid(0, 0)
            except os.error:
                pid = None

This is performance lag and can be improved. The collect_children can only wait/collect it's own children.
msg136953 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-05-26 12:46
In the common case, I don't think that the os.waitpid(0, 0) is the hot spot, but rather this:

for child in self.active_children:
    os.waitpid(child, os.WNOHANG)

It's called every time, and it's O(number of active children).

os.waitpid(0, 0) is only called when max_children (40) is reached. Also, I'm not sure why it's called without WNOHANG, because that means that we will block until active_children falls below max_children. That's an arbitrary limit, especially since it's not documented.

I've written a first draft patch using process group, that removes active_children and max_children.
The only slightly tricky part is the call to setpgid from the child and from the parent, to avoid a race where the child could try to join a stale PGID.
Note that I assume that setpgid is available on every Unix, since according to http://pubs.opengroup.org/onlinepubs/007908799/xsh/setpgid.html , it's "Derived from the POSIX.1-1988 standard".
Also, according to buildbots' configure logs, it's available everywhere.
msg136957 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-05-26 13:11
Some precisions:
1) Of course, if a handler changes its process group through setsid/setpgrp, it won't be waited on.
2) If a handler running on behalf of a process which is the current process group leader calls setsid, it will get an EPERM error.
I don't think anyone is using that, but I'd rather make it clear.
The only way I can think of to alleviate 2 would be to spawn a dummy process that would just be used as process group leader (and since it would keep running, no need to re-allocate a new PGID when worker processes exit).
msg223877 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-07-24 19:35
Closing as wont't fix, since we now have asyncio which handles this much better.
History
Date User Action Args
2022-04-11 14:57:17adminsetgithub: 56393
2014-07-24 19:35:35neologixsetstatus: open -> closed
resolution: wont fix
messages: + msg223877

stage: needs patch -> resolved
2012-03-27 14:37:26giampaolo.rodolasetnosy: + giampaolo.rodola
2011-05-26 13:11:09neologixsetmessages: + msg136957
2011-05-26 12:46:22neologixsetfiles: + ss_wait_group.diff
keywords: + patch
messages: + msg136953
2011-05-26 05:21:02orsenthilsetnosy: + neologix
2011-05-26 05:19:52orsenthilcreate