msg265929 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-05-20 10:19 |
The issue #26741 modified the subprocess.Popen destructor to emit a ResourceWarning if the child process is still running. According to Martin Panter, it can be deliberate to let the subprocess running if the management of the subprocess is delegated to a different object. I'm not convinced that it's safe to do that, but I chose to open this issue to discuss the feature. My fear is that subprocess.Popen handles many private objects required by the subprocess, not only its pid. For example, on Windows Popen stores the handle of the subprocess. Popen also contains pipe objects when stdin, stdout and/or stderr is redirected. I guess that detach() makes sense when the developer knows what he/she is doing and knows how to handle all resources attached to a subprocess. |
|
|
msg265934 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-05-20 11:12 |
Hum, I had to patch asyncio to avoid a ResourceWarning because asyncio polls the status of the child process using a child watcher, whereas the Popen object is not aware of the child watcher: New changeset 72946937536e by Victor Stinner in branch '3.5': asyncio: fix ResourceWarning related to subprocesses https://hg.python.org/cpython/rev/72946937536e The asyncio child watcher uses os.waitpid() on a specific pid or wait for the exit of any child process depending on the chosen implemetation. The fast watcher implementations uses a signal handler on the signal SIGCHLD. |
|
|
msg265941 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-05-20 12:54 |
One potential way to fix Issue 27069 (leave a webbrowser running in the background) might be to use a detach() method. Pseudocode: launcher = """\ import subprocess, sys proc = subprocess.Popen(sys.argv) # Popen object gives up “ownership” of child. # Caller exits straight afterwards, letting the OS take care of monitoring the child. proc.detach() """ def open_firefox(url): cmd = (sys.executable, "-c", launcher, "firefox", url) subprocess.check_call(cmd) |
|
|
msg265942 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-05-20 13:13 |
Martin Panter: > One potential way to fix Issue 27069 (leave a webbrowser running in the background) might be to use a detach() method. Pseudocode: Your code is a workaround to hide the warning, it doesn't fix the bug (creating a zombi process). |
|
|
msg265943 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-05-20 13:20 |
In this example, I think we want to drop the zombie so the “init” process will call wait() on it. My pseudocode is similar to your fork() suggestion in Issue 27069. Fork returns the child’s PID, but we would ignore the PID and exit the intermediate parent process instead. |
|
|
msg284933 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2017-01-07 20:57 |
The user can access pipes and close them directly, or keep their own reference. I don’t think detach() should touch pipes, and __exit__() should probably continue to close them. Maybe call the method detach_pid() if that makes it clearer that pipes are unaffected. For the Windows process handle, I suggest detach() should close it. I believe this would make Windows work like Unix when you set SIGCHLD to automatically reap children. |
|
|
msg297101 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-06-28 01:07 |
@Martin Panter: Do you still want this feature? Any progress on bpo-27069? |
|
|
msg297972 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2017-07-08 23:05 |
Personally, I haven’t needed this feature. But I still think it may be a decent solution for the “webbrowser” module, potentially your “asyncio” problem, and other cases that could now trigger an unwanted ResourceWarning. |
|
|
msg306874 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-11-24 01:34 |
Sorry, but I'm not sure that it's a good idea to add a detach() method. It's hard to follow who owns all resources attached to a process, not only it's pid, but also streams which may be buffered. I propose the close the issue. The webbrowser issue can be fixed differently without detach(). |
|
|