[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

Changes by Jack O'Connor : -- nosy: +oconnor663 ___ Python tracker ___ ___

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

STINNER Victor added the comment: > The one potential problem that I see is that it looks like specifying > PROC_THREAD_ATTRIBUTE_HANDLE_LIST means that no other handles are inherited, > even if they are inheritable. We can maybe add a parameter to configure the behaviour: keep current

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

Adam Meily added the comment: Attached is a test Python script that you can run to see the race condition in action. There are two Python scripts: pipe.py and reader.py. - pipe.py: make two subprocess.Popen() calls from two different threads. - reader.py: (its content is in the bottom

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

New submission from Adam Meily: ** This issue and attached patch only affect Windows ** Currently, the Popen constructor will duplicate any stdout, stdin, and/or stderr handle passed in and make them inheritable, by calling DuplicateHandle. If two threads call Popen at the same time, the

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

R. David Murray added the comment: Is it possible to construct a test for this? -- components: +Windows nosy: +gregory.p.smith, paul.moore, r.david.murray, steve.dower, tim.golden, zach.ware stage: - patch review versions: -Python 3.2, Python 3.3

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

Adam Meily added the comment: Ok, I can re-implement the patch to meet what you all are looking for. I just want to double check that I'm on the same page: I'll get rid of the lock, because the fix should really be done in the call to CreateProcessW. I imagine that I'll be editing

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

STINNER Victor added the comment: Yeah, when stdin, stdout or stderr is a pipe, subprocess.Popen() calls CreateProcess() with the STARTF_USESTDHANDLES flag and bInheritHandles=TRUE. This issue was fixed on UNIX for file descriptor inheritance. The fix is a major change: Python 3.4 now only

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

Adam Meily added the comment: @r.david.murray: Yes I could make a test. @haypo: I did not know about the PROC_THREAD_ATTRIBUTE_HANDLE_LIST structure, thanks for the heads up. You pointed me in the right direction, and I see now that you've been following this, and similar, subprocessing

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

STINNER Victor added the comment: Yeah, when stdin, stdout or stderr is a pipe, subprocess.Popen() calls CreateProcess() with the STARTF_USESTDHANDLES flag and bInheritHandles=TRUE. This issue was fixed on UNIX for file descriptor inheritance. The fix is a major change: Python 3.4 now only

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

STINNER Victor added the comment: It looks like the structure you reference to, PROC_THREAD_ATTRIBUTE_HANDLE_LIST (STARTUPINFOEX), is only available in Windows Vista and after. Which means that Windows XP/Server 2003 would still have this issue. Windows XP is no more supported in Python

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

Changes by STINNER Victor victor.stin...@gmail.com: -- Removed message: http://bugs.python.org/msg248964 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24909 ___

[issue24909] Windows: subprocess.Popen: race condition for leaking inheritable handles

Zachary Ware added the comment: STINNER Victor added the comment: For Windows Server 2003, yes, we will have to keep the current code which has the race condition. Server 2003 is also unsupported in 3.5+ (MS extended support ended in July). --