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.

Author meilyadam
Recipients meilyadam
Date 2015-08-21.16:51:49
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1440175915.18.0.538138459653.issue24909@psf.upfronthosting.co.za>
In-reply-to
Content
** 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 newly created inheritable handles will leak into the subprocess that's running being created in the opposite thread. This has consequences when two or more subprocesses are piped together and executed at the time time.


Example
=======

A pipe is created using the os.pipe() function. Then, two threads are started, T1 and T2. T1 calls Popen, setting stdout to the write-end of the pipe. T2 calls Popen, setting stdin to the read-end of the pipe. Stock CPython would make T1.stdout and T2.stdin inheritable. T1's Popen will create a subprocess, which correctly inherits T1.stdout, but it will also inherit the T2.stdin handle. Likewise, T2's Popen will create a subprocess, which correctly inherits T2.stdin, but it will also inherit the T1.stdout handle. Thus, in this situation where both ends of the pipe were accidentally inherited, the pipe will not properly close and T2's subprocess will have to be forcefully killed (assuming it is reading from stdin).

The patch simply enforces that the lifetime of an inheritable handle is limited to a single call to Popen.


Why this should be in CPython
=============================

I believe this change should be in CPython. Tracking down this bug into the Python core took a substantial amount of time and work. Can I do this in my Python application and achieve the same effect? Absolutely, but I would argue that I shouldn't have to. The fix in CPython is very minimal:

 - It won't break existing code. Even if my project already has a lock around calls to Popen, my code will continue to work. Sure, it's a duplicate lock, but nothing will break or deadlock.

 - The performance impact is negligible. The lock is only instantiated for Windows platforms and acquiring/releasing a lock appears to have very low overhead.

 - Because the patch is very small, it can be easily backported to other Python versions. As far as I can tell, Python 3.5, 3.4, 3.3, and 2.7 will work with the attached patch. The one difference between the Python versions is that subprocess.mswindows was renamed to subprocess._mswindows at some point, which is very minor.

 - Python 3.4's  new OS-specific inheritable functions [os.get_inheritable(), os.set_inheritable(), etc] will continue to work as expected. So, nothing is broken or changed there.

Old code won't break, new code will benefit immediately, and existing code will work better.


Similar Issues
==============

 - #19809: subprocess should warn uses on race conditions when multiple threads spawn child processes

 - #2320: Race condition in subprocess using stdin

 - #12739: read stuck with multithreading and simultaneous subprocess.Popen
History
Date User Action Args
2015-08-21 16:51:55meilyadamsetrecipients: + meilyadam
2015-08-21 16:51:55meilyadamsetmessageid: <1440175915.18.0.538138459653.issue24909@psf.upfronthosting.co.za>
2015-08-21 16:51:54meilyadamlinkissue24909 messages
2015-08-21 16:51:51meilyadamcreate