classification
Title: subprocess on windows leaks stdout/stderr handle to child process when stdout/stderr overridden
Type: resource usage Stage:
Components: Windows Versions: Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: paul.moore, saifujinaro, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords:

Created on 2016-04-10 20:21 by saifujinaro, last changed 2016-04-12 23:02 by vstinner.

Messages (3)
msg263149 - (view) Author: Sai (saifujinaro) Date: 2016-04-10 20:21
Tested in on Windows 8.1 with python 2.7.5.

I have a parent process that creates a child process and calls communicate to get stdout/stderr from the child.  The child process calls a persistent process, with stdout/stderr/stdin set to os.devnull, and then exits without waiting on the child process.  Sample code is below.

The child process exits successfully, but communicate on the the parent process does not return until the persistent process is terminated.  Expected behavior is that the child process closes its stdout/stderr pipes on exit, and those pipes are not open anywhere else, so the parent process returns from communicate once the child process exits.

One fix that stops the bug from manifesting is to edit subprocess.py:954 and pass in FALSE for inherit_handles in the call to _subprocess.CreateProcess rather than passing in int(not close_fds).  With the current code there is no way for the user of subprocess to trigger this behavior, because close_fds is necessarily False when redirecting stdout/stderr/stdin due to an exception raised in the Popen constructor.

I believe the proper fix to set close_fds to True, and pass in the handles through startupinfo, if any one of the pipes has been redirected.  This will require some changes to _get_handles and some significant testing.

A workaround fix that is easier to implement is to remove the assertion in the Popen constructor and allow the caller to specify close_fds=True even when redirecting one of the inputs.

Test case:
Three programs: parent.py, child.py, and persistent.py.
Launch parent.py.
Behavior:
child.py returns immediately
resident.py exits after 10 seconds
parent.py prints its output and exits immediately after resident.py exits

Expected Behavior:
child.py returns immediately
parent.py prints its output and exits immediately after child.py exits
resident.py exits after 10 seconds


############### parent.py ##########################
import subprocess

proc = subprocess.Popen("python child.py", stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
(output, error) = proc.communicate()
print 'parent complete'

############### child.py ###########################
import os
import subprocess

with open(os.devnull, 'w') as devnull:
    proc = subprocess.Popen('python resident.py', stdout=devnull, stderr=devnull, stdin=devnull)

############### resident.py ########################
import time
time.sleep(10)
msg263156 - (view) Author: Sai (saifujinaro) Date: 2016-04-10 22:58
You can workaround this problem by adding a middleman process that sets close_fds=True:

############### childworkaround.py #################
import os
import subprocess

with open(os.devnull, 'w') as devnull:
    script = "import subprocess; import sys; subprocess.Popen(sys.argv[1:], close_fds=True)"
    proc = subprocess.Popen(['python', '-c', script, 'python', 'resident.py'], stdout=devnull, stderr=devnull, stdin=devnull)
msg263283 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-12 23:02
The PEP 446 fixes the issue on UNIX for file descriptors, but you are right, there is still an issue with inheritable Windows handles. By default, Windows handles are not inheritable, but subprocess requires to make stdout and stderr pipes inheritable.

See:
https://www.python.org/dev/peps/pep-0446/#only-inherit-some-handles-on-windows

See also the issue #19764: "subprocess: use PROC_THREAD_ATTRIBUTE_HANDLE_LIST with STARTUPINFOEX on Windows Vista".
History
Date User Action Args
2016-04-12 23:02:50vstinnersettype: resource usage
2016-04-12 23:02:28vstinnersetnosy: + vstinner
messages: + msg263283
2016-04-10 22:58:44saifujinarosetmessages: + msg263156
2016-04-10 20:21:52saifujinarocreate