Martin Panter added the comment: I don’t know enough about process groups and sessions to properly review, but some things that stand out:
* Patch is missing documentation and tests * If the “killpg” just wraps os.killpg, perhaps adding the method is not justified * Are there any race conditions with killing a process group that has already exited. When does a process group get freed and potentially reused (so you may kill the wrong group)? The “send_signal” method (and others) have a check to avoid signalling an unrelated process. * The method is called killpg, and the doc string mentions SIGKILL, but the implementation says SIGTERM * What happens if you use killpg without starting a new session? If it kills the parent process as well, that sounds like a source of subtle bugs that may only be detected in unexpected cases (e.g. Ctrl+C or timeout) * Be aware of Issue 25942. It is not clear what should happen to the child process(es) when the timeout happens, or when the “communicate” call is interrupted. * What platforms does this support and what happens if there is no platform support? ---------- nosy: +martin.panter stage: -> patch review type: behavior -> enhancement versions: +Python 3.7 -Python 3.5, Python 3.6 _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue26534> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
