issue7213: Open the pipes used by subprocesses with the FD_CLOEXEC flag from
the C code, using pipe2() when available. Adds unittests for close_fds and cloexec behaviors.
This commit is contained in:
Gregory P. Smith
parent
f560485388
commit
51ee270876
|
|
@ -390,6 +390,23 @@ else:
|
|||
# POSIX defines PIPE_BUF as >= 512.
|
||||
_PIPE_BUF = getattr(select, 'PIPE_BUF', 512)
|
||||
|
||||
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)
|
||||
|
||||
return fds
|
||||
|
||||
__all__ = ["Popen", "PIPE", "STDOUT", "call", "check_call", "getstatusoutput",
|
||||
"getoutput", "check_output", "CalledProcessError"]
|
||||
|
|
@ -1031,7 +1048,7 @@ class Popen(object):
|
|||
if stdin is None:
|
||||
pass
|
||||
elif stdin == PIPE:
|
||||
p2cread, p2cwrite = os.pipe()
|
||||
p2cread, p2cwrite = _create_pipe()
|
||||
elif isinstance(stdin, int):
|
||||
p2cread = stdin
|
||||
else:
|
||||
|
|
@ -1041,7 +1058,7 @@ class Popen(object):
|
|||
if stdout is None:
|
||||
pass
|
||||
elif stdout == PIPE:
|
||||
c2pread, c2pwrite = os.pipe()
|
||||
c2pread, c2pwrite = _create_pipe()
|
||||
elif isinstance(stdout, int):
|
||||
c2pwrite = stdout
|
||||
else:
|
||||
|
|
@ -1051,7 +1068,7 @@ class Popen(object):
|
|||
if stderr is None:
|
||||
pass
|
||||
elif stderr == PIPE:
|
||||
errread, errwrite = os.pipe()
|
||||
errread, errwrite = _create_pipe()
|
||||
elif stderr == STDOUT:
|
||||
errwrite = c2pwrite
|
||||
elif isinstance(stderr, int):
|
||||
|
|
@ -1065,16 +1082,6 @@ class Popen(object):
|
|||
errread, errwrite)
|
||||
|
||||
|
||||
def _set_cloexec_flag(self, fd):
|
||||
try:
|
||||
cloexec_flag = fcntl.FD_CLOEXEC
|
||||
except AttributeError:
|
||||
cloexec_flag = 1
|
||||
|
||||
old = fcntl.fcntl(fd, fcntl.F_GETFD)
|
||||
fcntl.fcntl(fd, fcntl.F_SETFD, old | cloexec_flag)
|
||||
|
||||
|
||||
def _close_fds(self, but):
|
||||
os.closerange(3, but)
|
||||
os.closerange(but + 1, MAXFD)
|
||||
|
|
@ -1116,10 +1123,9 @@ class Popen(object):
|
|||
# For transferring possible exec failure from child to parent.
|
||||
# Data format: "exception name:hex errno:description"
|
||||
# Pickle is not used; it is complex and involves memory allocation.
|
||||
errpipe_read, errpipe_write = os.pipe()
|
||||
errpipe_read, errpipe_write = _create_pipe()
|
||||
try:
|
||||
try:
|
||||
self._set_cloexec_flag(errpipe_write)
|
||||
|
||||
if _posixsubprocess:
|
||||
# We must avoid complex work that could involve
|
||||
|
|
|
|||
|
|
@ -0,0 +1,24 @@
|
|||
"""When called as a script, print a comma-separated list of the open
|
||||
file descriptors on stdout."""
|
||||
|
||||
import errno
|
||||
import os
|
||||
import fcntl
|
||||
|
||||
try:
|
||||
_MAXFD = os.sysconf("SC_OPEN_MAX")
|
||||
except:
|
||||
_MAXFD = 256
|
||||
|
||||
def isopen(fd):
|
||||
"""Return True if the fd is open, and False otherwise"""
|
||||
try:
|
||||
fcntl.fcntl(fd, fcntl.F_GETFD, 0)
|
||||
except IOError as e:
|
||||
if e.errno == errno.EBADF:
|
||||
return False
|
||||
raise
|
||||
return True
|
||||
|
||||
if __name__ == "__main__":
|
||||
print(','.join(str(fd) for fd in range(0, _MAXFD) if isopen(fd)))
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
"""When called as a script, consumes the input"""
|
||||
|
||||
import sys
|
||||
|
||||
if __name__ = "__main__":
|
||||
for line in sys.stdin:
|
||||
pass
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
"""When ran as a script, simulates cat with no arguments."""
|
||||
|
||||
import sys
|
||||
|
||||
if __name__ == "__main__":
|
||||
for line in sys.stdin:
|
||||
sys.stdout.write(line)
|
||||
|
|
@ -0,0 +1,10 @@
|
|||
"""When called with a single argument, simulated fgrep with a single
|
||||
argument and no options."""
|
||||
|
||||
import sys
|
||||
|
||||
if __name__ == "__main__":
|
||||
pattern = sys.argv[1]
|
||||
for line in sys.stdin:
|
||||
if pattern in line:
|
||||
sys.stdout.write(line)
|
||||
|
|
@ -10,6 +10,7 @@ import time
|
|||
import re
|
||||
import sysconfig
|
||||
import warnings
|
||||
import select
|
||||
try:
|
||||
import gc
|
||||
except ImportError:
|
||||
|
|
@ -964,6 +965,83 @@ class POSIXProcessTestCase(BaseTestCase):
|
|||
exitcode = subprocess.call([program, "-c", "pass"], env=envb)
|
||||
self.assertEqual(exitcode, 0)
|
||||
|
||||
def test_pipe_cloexec(self):
|
||||
sleeper = support.findfile("input_reader.py", subdir="subprocessdata")
|
||||
fd_status = support.findfile("fd_status.py", subdir="subprocessdata")
|
||||
|
||||
p1 = subprocess.Popen([sys.executable, sleeper],
|
||||
stdin=subprocess.PIPE, stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE, close_fds=False)
|
||||
|
||||
self.addCleanup(p1.communicate, b'')
|
||||
|
||||
p2 = subprocess.Popen([sys.executable, fd_status],
|
||||
stdout=subprocess.PIPE, close_fds=False)
|
||||
|
||||
output, error = p2.communicate()
|
||||
result_fds = set(map(int, output.split(b',')))
|
||||
unwanted_fds = set([p1.stdin.fileno(), p1.stdout.fileno(),
|
||||
p1.stderr.fileno()])
|
||||
|
||||
self.assertFalse(result_fds & unwanted_fds,
|
||||
"Expected no fds from %r to be open in child, "
|
||||
"found %r" %
|
||||
(unwanted_fds, result_fds & unwanted_fds))
|
||||
|
||||
def test_pipe_cloexec_real_tools(self):
|
||||
qcat = support.findfile("qcat.py", subdir="subprocessdata")
|
||||
qgrep = support.findfile("qgrep.py", subdir="subprocessdata")
|
||||
|
||||
subdata = b'zxcvbn'
|
||||
data = subdata * 4 + b'\n'
|
||||
|
||||
p1 = subprocess.Popen([sys.executable, qcat],
|
||||
stdin=subprocess.PIPE, stdout=subprocess.PIPE,
|
||||
close_fds=False)
|
||||
|
||||
p2 = subprocess.Popen([sys.executable, qgrep, subdata],
|
||||
stdin=p1.stdout, stdout=subprocess.PIPE,
|
||||
close_fds=False)
|
||||
|
||||
self.addCleanup(p1.wait)
|
||||
self.addCleanup(p2.wait)
|
||||
self.addCleanup(p1.terminate)
|
||||
self.addCleanup(p2.terminate)
|
||||
|
||||
p1.stdin.write(data)
|
||||
p1.stdin.close()
|
||||
|
||||
readfiles, ignored1, ignored2 = select.select([p2.stdout], [], [], 10)
|
||||
|
||||
self.assertTrue(readfiles, "The child hung")
|
||||
self.assertEqual(p2.stdout.read(), data)
|
||||
|
||||
def test_close_fds(self):
|
||||
fd_status = support.findfile("fd_status.py", subdir="subprocessdata")
|
||||
|
||||
fds = os.pipe()
|
||||
self.addCleanup(os.close, fds[0])
|
||||
self.addCleanup(os.close, fds[1])
|
||||
|
||||
open_fds = set(fds)
|
||||
|
||||
p = subprocess.Popen([sys.executable, fd_status],
|
||||
stdout=subprocess.PIPE, close_fds=False)
|
||||
output, ignored = p.communicate()
|
||||
remaining_fds = set(map(int, output.split(b',')))
|
||||
|
||||
self.assertEqual(remaining_fds & open_fds, open_fds,
|
||||
"Some fds were closed")
|
||||
|
||||
p = subprocess.Popen([sys.executable, fd_status],
|
||||
stdout=subprocess.PIPE, close_fds=True)
|
||||
output, ignored = p.communicate()
|
||||
remaining_fds = set(map(int, output.split(b',')))
|
||||
|
||||
self.assertFalse(remaining_fds & open_fds,
|
||||
"Some fds were left open")
|
||||
self.assertIn(1, remaining_fds, "Subprocess failed")
|
||||
|
||||
|
||||
@unittest.skipUnless(mswindows, "Windows specific tests")
|
||||
class Win32ProcessTestCase(BaseTestCase):
|
||||
|
|
|
|||
|
|
@ -888,7 +888,7 @@ MACHDEPS= $(PLATDIR) $(EXTRAPLATDIR)
|
|||
XMLLIBSUBDIRS= xml xml/dom xml/etree xml/parsers xml/sax
|
||||
LIBSUBDIRS= tkinter tkinter/test tkinter/test/test_tkinter \
|
||||
tkinter/test/test_ttk site-packages test \
|
||||
test/decimaltestdata test/xmltestdata \
|
||||
test/decimaltestdata test/xmltestdata test/subprocessdata \
|
||||
test/tracedmodules test/encoded_modules \
|
||||
concurrent concurrent/futures encodings \
|
||||
email email/mime email/test email/test/data \
|
||||
|
|
|
|||
|
|
@ -1,6 +1,10 @@
|
|||
/* Authors: Gregory P. Smith & Jeffrey Yasskin */
|
||||
#include "Python.h"
|
||||
#ifdef HAVE_PIPE2
|
||||
#define _GNU_SOURCE
|
||||
#endif
|
||||
#include <unistd.h>
|
||||
#include <fcntl.h>
|
||||
|
||||
|
||||
#define POSIX_CALL(call) if ((call) == -1) goto error
|
||||
|
|
@ -398,6 +402,45 @@ Returns: the child process's PID.\n\
|
|||
Raises: Only on an error in the parent process.\n\
|
||||
");
|
||||
|
||||
PyDoc_STRVAR(subprocess_cloexec_pipe_doc,
|
||||
"cloexec_pipe() -> (read_end, write_end)\n\n\
|
||||
Create a pipe whose ends have the cloexec flag set.");
|
||||
|
||||
static PyObject *
|
||||
subprocess_cloexec_pipe(PyObject *self, PyObject *noargs)
|
||||
{
|
||||
int fds[2];
|
||||
int res;
|
||||
#ifdef HAVE_PIPE2
|
||||
Py_BEGIN_ALLOW_THREADS
|
||||
res = pipe2(fds, O_CLOEXEC);
|
||||
Py_END_ALLOW_THREADS
|
||||
#else
|
||||
/* We hold the GIL which offers some protection from other code calling
|
||||
* fork() before the CLOEXEC flags have been set but we can't guarantee
|
||||
* anything without pipe2(). */
|
||||
long oldflags;
|
||||
|
||||
res = pipe(fds);
|
||||
|
||||
if (res == 0) {
|
||||
oldflags = fcntl(fds[0], F_GETFD, 0);
|
||||
if (oldflags < 0) res = oldflags;
|
||||
}
|
||||
if (res == 0)
|
||||
res = fcntl(fds[0], F_SETFD, oldflags | FD_CLOEXEC);
|
||||
|
||||
if (res == 0) {
|
||||
oldflags = fcntl(fds[1], F_GETFD, 0);
|
||||
if (oldflags < 0) res = oldflags;
|
||||
}
|
||||
if (res == 0)
|
||||
res = fcntl(fds[1], F_SETFD, oldflags | FD_CLOEXEC);
|
||||
#endif
|
||||
if (res != 0)
|
||||
return PyErr_SetFromErrno(PyExc_OSError);
|
||||
return Py_BuildValue("(ii)", fds[0], fds[1]);
|
||||
}
|
||||
|
||||
/* module level code ********************************************************/
|
||||
|
||||
|
|
@ -407,6 +450,7 @@ PyDoc_STRVAR(module_doc,
|
|||
|
||||
static PyMethodDef module_methods[] = {
|
||||
{"fork_exec", subprocess_fork_exec, METH_VARARGS, subprocess_fork_exec_doc},
|
||||
{"cloexec_pipe", subprocess_cloexec_pipe, METH_NOARGS, subprocess_cloexec_pipe_doc},
|
||||
{NULL, NULL} /* sentinel */
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -1035,6 +1035,8 @@ def add_files(db):
|
|||
if dir=='xmltestdata':
|
||||
lib.glob("*.xml")
|
||||
lib.add_file("test.xml.out")
|
||||
if dir=='subprocessdata':
|
||||
lib.glob("*.py")
|
||||
if dir=='output':
|
||||
lib.glob("test_*")
|
||||
if dir=='sndhdrdata':
|
||||
|
|
|
|||
|
|
@ -4235,7 +4235,7 @@ case $ac_sys_system in
|
|||
OSF*) AC_MSG_ERROR(OSF* systems are deprecated unless somebody volunteers. Check http://bugs.python.org/issue8606) ;;
|
||||
esac
|
||||
|
||||
|
||||
AC_CHECK_FUNC(pipe2, AC_DEFINE(HAVE_PIPE2, 1, [Define if the OS supports pipe2()]), )
|
||||
|
||||
AC_SUBST(THREADHEADERS)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue