Issue #10806, issue #9905: Fix subprocess pipes when some of the standard

file descriptors (0, 1, 2) are closed in the parent process.  Initial
patch by Ross Lagerwall.
This commit is contained in:
Antoine Pitrou 2011-01-03 18:23:55 +00:00
parent 63ebe1c309
commit c9c83ba896
4 changed files with 104 additions and 33 deletions

View File

@ -393,22 +393,22 @@ else:
# POSIX defines PIPE_BUF as >= 512.
_PIPE_BUF = getattr(select, 'PIPE_BUF', 512)
_FD_CLOEXEC = getattr(fcntl, 'FD_CLOEXEC', 1)
def _set_cloexec(fd, cloexec):
old = fcntl.fcntl(fd, fcntl.F_GETFD)
if cloexec:
fcntl.fcntl(fd, fcntl.F_SETFD, old | _FD_CLOEXEC)
else:
fcntl.fcntl(fd, fcntl.F_SETFD, old & ~_FD_CLOEXEC)
if _posixsubprocess:
_create_pipe = _posixsubprocess.cloexec_pipe
else:
def _create_pipe():
try:
cloexec_flag = fcntl.FD_CLOEXEC
except AttributeError:
cloexec_flag = 1
fds = os.pipe()
old = fcntl.fcntl(fds[0], fcntl.F_GETFD)
fcntl.fcntl(fds[0], fcntl.F_SETFD, old | cloexec_flag)
old = fcntl.fcntl(fds[1], fcntl.F_GETFD)
fcntl.fcntl(fds[1], fcntl.F_SETFD, old | cloexec_flag)
_set_cloexec(fds[0], True)
_set_cloexec(fds[1], True)
return fds
__all__ = ["Popen", "PIPE", "STDOUT", "call", "check_call", "getstatusoutput",
@ -1194,23 +1194,25 @@ class Popen(object):
os.close(errpipe_read)
# Dup fds for child
if p2cread != -1:
os.dup2(p2cread, 0)
if c2pwrite != -1:
os.dup2(c2pwrite, 1)
if errwrite != -1:
os.dup2(errwrite, 2)
def _dup2(a, b):
# dup2() removes the CLOEXEC flag but
# we must do it ourselves if dup2()
# would be a no-op (issue #10806).
if a == b:
_set_cloexec(a, False)
elif a != -1:
os.dup2(a, b)
_dup2(p2cread, 0)
_dup2(c2pwrite, 1)
_dup2(errwrite, 2)
# Close pipe fds. Make sure we don't close the
# same fd more than once, or standard fds.
if p2cread != -1 and p2cread not in (0,):
os.close(p2cread)
if (c2pwrite != -1 and
c2pwrite not in (p2cread, 1)):
os.close(c2pwrite)
if (errwrite != -1 and
errwrite not in (p2cread, c2pwrite, 2)):
os.close(errwrite)
closed = set()
for fd in [p2cread, c2pwrite, errwrite]:
if fd > 2 and fd not in closed:
os.close(fd)
closed.add(fd)
# Close all other fds, if asked for
if close_fds:

View File

@ -903,6 +903,58 @@ class POSIXProcessTestCase(BaseTestCase):
self.assertStderrEqual(stderr, b'')
self.assertEqual(p.wait(), -signal.SIGTERM)
def check_close_std_fds(self, fds):
# Issue #9905: test that subprocess pipes still work properly with
# some standard fds closed
stdin = 0
newfds = []
for a in fds:
b = os.dup(a)
newfds.append(b)
if a == 0:
stdin = b
try:
for fd in fds:
os.close(fd)
out, err = subprocess.Popen([sys.executable, "-c",
'import sys;'
'sys.stdout.write("apple");'
'sys.stdout.flush();'
'sys.stderr.write("orange")'],
stdin=stdin,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE).communicate()
err = support.strip_python_stderr(err)
self.assertEqual((out, err), (b'apple', b'orange'))
finally:
for b, a in zip(newfds, fds):
os.dup2(b, a)
for b in newfds:
os.close(b)
def test_close_fd_0(self):
self.check_close_std_fds([0])
def test_close_fd_1(self):
self.check_close_std_fds([1])
def test_close_fd_2(self):
self.check_close_std_fds([2])
def test_close_fds_0_1(self):
self.check_close_std_fds([0, 1])
def test_close_fds_0_2(self):
self.check_close_std_fds([0, 2])
def test_close_fds_1_2(self):
self.check_close_std_fds([1, 2])
def test_close_fds_0_1_2(self):
# Issue #10806: test that subprocess pipes still work properly with
# all standard fds closed.
self.check_close_std_fds([0, 1, 2])
def test_surrogates_error_message(self):
def prepare():
raise ValueError("surrogate:\uDCff")

View File

@ -23,6 +23,10 @@ Core and Builtins
Library
-------
- Issue #10806, issue #9905: Fix subprocess pipes when some of the standard
file descriptors (0, 1, 2) are closed in the parent process. Initial
patch by Ross Lagerwall.
- `unittest.TestCase` can be instantiated without a method name; for simpler
exploration from the interactive interpreter.

View File

@ -69,27 +69,40 @@ static void child_exec(char *const exec_array[],
}
POSIX_CALL(close(errpipe_read));
/* Dup fds for child. */
if (p2cread != -1) {
/* Dup fds for child.
dup2() removes the CLOEXEC flag but we must do it ourselves if dup2()
would be a no-op (issue #10806). */
if (p2cread == 0) {
int old = fcntl(p2cread, F_GETFD);
if (old != -1)
fcntl(p2cread, F_SETFD, old & ~FD_CLOEXEC);
} else if (p2cread != -1) {
POSIX_CALL(dup2(p2cread, 0)); /* stdin */
}
if (c2pwrite != -1) {
if (c2pwrite == 1) {
int old = fcntl(c2pwrite, F_GETFD);
if (old != -1)
fcntl(c2pwrite, F_SETFD, old & ~FD_CLOEXEC);
} else if (c2pwrite != -1) {
POSIX_CALL(dup2(c2pwrite, 1)); /* stdout */
}
if (errwrite != -1) {
if (errwrite == 2) {
int old = fcntl(errwrite, F_GETFD);
if (old != -1)
fcntl(errwrite, F_SETFD, old & ~FD_CLOEXEC);
} else if (errwrite != -1) {
POSIX_CALL(dup2(errwrite, 2)); /* stderr */
}
/* Close pipe fds. Make sure we don't close the same fd more than */
/* once, or standard fds. */
if (p2cread != -1 && p2cread != 0) {
if (p2cread > 2) {
POSIX_CALL(close(p2cread));
}
if (c2pwrite != -1 && c2pwrite != p2cread && c2pwrite != 1) {
if (c2pwrite > 2) {
POSIX_CALL(close(c2pwrite));
}
if (errwrite != -1 && errwrite != p2cread &&
errwrite != c2pwrite && errwrite != 2) {
if (errwrite != c2pwrite && errwrite > 2) {
POSIX_CALL(close(errwrite));
}