homepage

There is an issue where if a python program closes all the std. file descriptors (e.g. a daemon) and then uses the subprocess module, the file descriptors may not be set up properly in the subprocess. This may actually be a fairly common use case in a daemon program that needs to run a subprocess and set up pipes to it.
Here is an example:
import os, subprocess, sys
x=os.open('/dev/null', os.O_RDWR)
os.close(0)
os.close(1)
os.close(2)
res = subprocess.Popen([sys.executable, "-c",
 'import sys;'
 'sys.stdout.write("apple");'
 'sys.stdout.flush();'
 'sys.stderr.write("orange")'],
 stdin=x,
 stdout=subprocess.PIPE,
 stderr=subprocess.PIPE).communicate()
with open('/tmp/out', 'w') as f:
 f.write(repr(res) + '\n')
 f.write(repr((b'apple', b'orange')) + '\n')
The expected output in /tmp/out is:
('apple', 'orange')
('apple', 'orange')
but we get:
(b'', b"Fatal Python error: Py_Initialize: can't initialize sys standard streams\nOSError: [Errno 9] Bad file descriptor\n")
(b'apple', b'orange')
The problem comes about where the calls are made (this applies to the python & c versions):
 os.dup2(p2cread, 0)
 os.dup2(c2pwrite, 1)
 os.dup2(errwrite, 2)
if c2pwrite or p2cread or errwrite is the same as what it's being dupped() to (eg if c2pwrite == 1) then the dup2 call does nothing. But, if we're using pipes, the close-on-exec flags are set initially and the dup2() call would normally remove the flag but it doesn't.
Attached is a patch which basically uses fcntl if necessary to remove the close-on-exec flag, and tests.

> Attached is a patch which basically uses fcntl if necessary to remove
> the close-on-exec flag, and tests.
Python adds extra output at the end of stderr when compiled in debug
mode. Therefore you first have to strip that output, like this:
 out, err = subprocess.Popen(...).communicate()
 err = support.strip_python_stderr(err)
 self.assertEqual((out, err), (b'apple', b'orange'))
Otherwise, looks good, thank you.

Updated patch for debug mode. Does this also need to be applied for 3.1?

> Updated patch for debug mode. Does this also need to be applied for 3.1?
Yes, but we can port it ourselves (unless you're really motivated)

Patch works fine, thank you. Here is an attempt at a slightly more readable code by refactoring.

This further patch also addresses issue9905 (incorporating Ross' tests).

See also #6610 which has a patch with another test.

This new patch makes tests more comprehensive (closes all combinations of the three standard fds).

Committed in r87695 (3.2), r87696 (3.1) and r87697 (2.7).