Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

bpo-20104: Improve error handling and fix a reference leak in os.posix_spawn(). (#6332)

This commit is contained in:
Serhiy Storchaka 2018-05-01 16:45:04 +03:00 committed by GitHub
parent 7508a54c77
commit ef347535f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 313 additions and 152 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
Doc/library/os.rst
View File

@ -3365,25 +3365,41 @@ written in Python, such as a mail server's external command delivery program.
.. function:: posix_spawn(path, argv, env, file_actions=None)
Wraps the posix_spawn() C library API for use from Python.
Wraps the :c:func:`posix_spawn` C library API for use from Python.
Most users should use :class:`subprocess.run` instead of posix_spawn.
Most users should use :func:`subprocess.run` instead of :func:`posix_spawn`.
The *path*, *args*, and *env* arguments are similar to :func:`execve`.
The *file_actions* argument may be a sequence of tuples describing actions
to take on specific file descriptors in the child process between the C
library implementation's fork and exec steps. The first item in each tuple
must be one of the three type indicator listed below describing the
remaining tuple elements:
library implementation's :c:func:`fork` and :c:func:`exec` steps.
The first item in each tuple must be one of the three type indicator
listed below describing the remaining tuple elements:
(os.POSIX_SPAWN_OPEN, fd, path, open flags, mode)
(os.POSIX_SPAWN_CLOSE, fd)
(os.POSIX_SPAWN_DUP2, fd, new_fd)
.. data:: POSIX_SPAWN_OPEN
These tuples correspond to the C library posix_spawn_file_actions_addopen,
posix_spawn_file_actions_addclose, and posix_spawn_file_actions_adddup2 API
calls used to prepare for the posix_spawn call itself.
(``os.POSIX_SPAWN_OPEN``, *fd*, *path*, *flags*, *mode*)
Performs ``os.dup2(os.open(path, flags, mode), fd)``.
.. data:: POSIX_SPAWN_CLOSE
(``os.POSIX_SPAWN_CLOSE``, *fd*)
Performs ``os.close(fd)``.
.. data:: POSIX_SPAWN_DUP2
(``os.POSIX_SPAWN_DUP2``, *fd*, *new_fd*)
Performs ``os.dup2(fd, new_fd)``.
These tuples correspond to the C library
:c:func:`posix_spawn_file_actions_addopen`,
:c:func:`posix_spawn_file_actions_addclose`, and
:c:func:`posix_spawn_file_actions_adddup2` API calls used to prepare
for the :c:func:`posix_spawn` call itself.
.. versionadded:: 3.7

177
Lib/test/test_posix.py
View File

@ -177,22 +177,6 @@ class PosixTester(unittest.TestCase):
os.close(fp)
@unittest.skipUnless(hasattr(os, 'posix_spawn'), "test needs os.posix_spawn")
def test_posix_spawn(self):
pid = posix.posix_spawn(sys.executable, [sys.executable, "-c", "pass"], os.environ,[])
self.assertEqual(os.waitpid(pid,0),(pid,0))
@unittest.skipUnless(hasattr(os, 'posix_spawn'), "test needs os.posix_spawn")
def test_posix_spawn_file_actions(self):
file_actions = []
file_actions.append((0,3,os.path.realpath(__file__),0,0))
file_actions.append((os.POSIX_SPAWN_CLOSE,2))
file_actions.append((os.POSIX_SPAWN_DUP2,1,4))
pid = posix.posix_spawn(sys.executable, [sys.executable, "-c", "pass"], os.environ, file_actions)
self.assertEqual(os.waitpid(pid,0),(pid,0))
@unittest.skipUnless(hasattr(posix, 'waitid'), "test needs posix.waitid()")
@unittest.skipUnless(hasattr(os, 'fork'), "test needs os.fork()")
def test_waitid(self):
@ -1437,9 +1421,168 @@ class PosixGroupsTester(unittest.TestCase):
posix.setgroups(groups)
self.assertListEqual(groups, posix.getgroups())
@unittest.skipUnless(hasattr(os, 'posix_spawn'), "test needs os.posix_spawn")
class TestPosixSpawn(unittest.TestCase):
def test_returns_pid(self):
pidfile = support.TESTFN
self.addCleanup(support.unlink, pidfile)
script = f"""if 1:
import os
with open({pidfile!r}, "w") as pidfile:
pidfile.write(str(os.getpid()))
"""
pid = posix.posix_spawn(sys.executable,
[sys.executable, '-c', script],
os.environ)
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
with open(pidfile) as f:
self.assertEqual(f.read(), str(pid))
def test_no_such_executable(self):
no_such_executable = 'no_such_executable'
try:
pid = posix.posix_spawn(no_such_executable,
[no_such_executable],
os.environ)
except FileNotFoundError as exc:
self.assertEqual(exc.filename, no_such_executable)
else:
pid2, status = os.waitpid(pid, 0)
self.assertEqual(pid2, pid)
self.assertNotEqual(status, 0)
def test_specify_environment(self):
envfile = support.TESTFN
self.addCleanup(support.unlink, envfile)
script = f"""if 1:
import os
with open({envfile!r}, "w") as envfile:
envfile.write(os.environ['foo'])
"""
pid = posix.posix_spawn(sys.executable,
[sys.executable, '-c', script],
{'foo': 'bar'})
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
with open(envfile) as f:
self.assertEqual(f.read(), 'bar')
def test_empty_file_actions(self):
pid = posix.posix_spawn(
sys.executable,
[sys.executable, '-c', 'pass'],
os.environ,
[]
)
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
def test_multiple_file_actions(self):
file_actions = [
(os.POSIX_SPAWN_OPEN, 3, os.path.realpath(__file__), os.O_RDONLY, 0),
(os.POSIX_SPAWN_CLOSE, 0),
(os.POSIX_SPAWN_DUP2, 1, 4),
]
pid = posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, file_actions)
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
def test_bad_file_actions(self):
with self.assertRaises(TypeError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, [None])
with self.assertRaises(TypeError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, [()])
with self.assertRaises(TypeError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, [(None,)])
with self.assertRaises(TypeError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, [(12345,)])
with self.assertRaises(TypeError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, [(os.POSIX_SPAWN_CLOSE,)])
with self.assertRaises(TypeError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, [(os.POSIX_SPAWN_CLOSE, 1, 2)])
with self.assertRaises(TypeError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ, [(os.POSIX_SPAWN_CLOSE, None)])
with self.assertRaises(ValueError):
posix.posix_spawn(sys.executable,
[sys.executable, "-c", "pass"],
os.environ,
[(os.POSIX_SPAWN_OPEN, 3, __file__ + '\0',
os.O_RDONLY, 0)])
def test_open_file(self):
outfile = support.TESTFN
self.addCleanup(support.unlink, outfile)
script = """if 1:
import sys
sys.stdout.write("hello")
"""
file_actions = [
(os.POSIX_SPAWN_OPEN, 1, outfile,
os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
stat.S_IRUSR | stat.S_IWUSR),
]
pid = posix.posix_spawn(sys.executable,
[sys.executable, '-c', script],
os.environ, file_actions)
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
with open(outfile) as f:
self.assertEqual(f.read(), 'hello')
def test_close_file(self):
closefile = support.TESTFN
self.addCleanup(support.unlink, closefile)
script = f"""if 1:
import os
try:
os.fstat(0)
except OSError as e:
with open({closefile!r}, 'w') as closefile:
closefile.write('is closed %d' % e.errno)
"""
pid = posix.posix_spawn(sys.executable,
[sys.executable, '-c', script],
os.environ,
[(os.POSIX_SPAWN_CLOSE, 0),])
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
with open(closefile) as f:
self.assertEqual(f.read(), 'is closed %d' % errno.EBADF)
def test_dup2(self):
dupfile = support.TESTFN
self.addCleanup(support.unlink, dupfile)
script = """if 1:
import sys
sys.stdout.write("hello")
"""
with open(dupfile, "wb") as childfile:
file_actions = [
(os.POSIX_SPAWN_DUP2, childfile.fileno(), 1),
]
pid = posix.posix_spawn(sys.executable,
[sys.executable, '-c', script],
os.environ, file_actions)
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
with open(dupfile) as f:
self.assertEqual(f.read(), 'hello')
def test_main():
try:
support.run_unittest(PosixTester, PosixGroupsTester)
support.run_unittest(PosixTester, PosixGroupsTester, TestPosixSpawn)
finally:
support.reap_children()

1
Misc/NEWS.d/next/Library/2018-04-01-19-21-04.bpo-20104.-AKcGa.rst Normal file
View File

@ -0,0 +1 @@
Improved error handling and fixed a reference leak in :func:`os.posix_spawn()`.

4
Modules/clinic/posixmodule.c.h
View File

@ -1742,7 +1742,7 @@ PyDoc_STRVAR(os_posix_spawn__doc__,
" env\n"
" Dictionary of strings mapping to strings.\n"
" file_actions\n"
" FileActions object.");
" A sequence of file action tuples.");
#define OS_POSIX_SPAWN_METHODDEF \
{"posix_spawn", (PyCFunction)os_posix_spawn, METH_FASTCALL, os_posix_spawn__doc__},
@ -6589,4 +6589,4 @@ exit:
#ifndef OS_GETRANDOM_METHODDEF
#define OS_GETRANDOM_METHODDEF
#endif /* !defined(OS_GETRANDOM_METHODDEF) */
/*[clinic end generated code: output=fc603214822bdda6 input=a9049054013a1b77]*/
/*[clinic end generated code: output=8d3d9dddf254c3c2 input=a9049054013a1b77]*/

245
Modules/posixmodule.c
View File

@ -5114,6 +5114,111 @@ enum posix_spawn_file_actions_identifier {
POSIX_SPAWN_DUP2
};
static int
parse_file_actions(PyObject *file_actions,
posix_spawn_file_actions_t *file_actionsp)
{
PyObject *seq;
PyObject *file_action = NULL;
PyObject *tag_obj;
seq = PySequence_Fast(file_actions,
"file_actions must be a sequence or None");
if (seq == NULL) {
return -1;
}
errno = posix_spawn_file_actions_init(file_actionsp);
if (errno) {
posix_error();
Py_DECREF(seq);
return -1;
}
for (int i = 0; i < PySequence_Fast_GET_SIZE(seq); ++i) {
file_action = PySequence_Fast_GET_ITEM(seq, i);
Py_INCREF(file_action);
if (!PyTuple_Check(file_action) || !PyTuple_GET_SIZE(file_action)) {
PyErr_SetString(PyExc_TypeError,
"Each file_actions element must be a non-empty tuple");
goto fail;
}
long tag = PyLong_AsLong(PyTuple_GET_ITEM(file_action, 0));
if (tag == -1 && PyErr_Occurred()) {
goto fail;
}
/* Populate the file_actions object */
switch (tag) {
case POSIX_SPAWN_OPEN: {
int fd, oflag;
PyObject *path;
unsigned long mode;
if (!PyArg_ParseTuple(file_action, "OiO&ik"
";A open file_action tuple must have 5 elements",
&tag_obj, &fd, PyUnicode_FSConverter, &path,
&oflag, &mode))
{
goto fail;
}
errno = posix_spawn_file_actions_addopen(file_actionsp,
fd, PyBytes_AS_STRING(path), oflag, (mode_t)mode);
Py_DECREF(path); /* addopen copied it. */
if (errno) {
posix_error();
goto fail;
}
break;
}
case POSIX_SPAWN_CLOSE: {
int fd;
if (!PyArg_ParseTuple(file_action, "Oi"
";A close file_action tuple must have 2 elements",
&tag_obj, &fd))
{
goto fail;
}
errno = posix_spawn_file_actions_addclose(file_actionsp, fd);
if (errno) {
posix_error();
goto fail;
}
break;
}
case POSIX_SPAWN_DUP2: {
int fd1, fd2;
if (!PyArg_ParseTuple(file_action, "Oii"
";A dup2 file_action tuple must have 3 elements",
&tag_obj, &fd1, &fd2))
{
goto fail;
}
errno = posix_spawn_file_actions_adddup2(file_actionsp,
fd1, fd2);
if (errno) {
posix_error();
goto fail;
}
break;
}
default: {
PyErr_SetString(PyExc_TypeError,
"Unknown file_actions identifier");
goto fail;
}
}
Py_DECREF(file_action);
}
Py_DECREF(seq);
return 0;
fail:
Py_DECREF(seq);
Py_DECREF(file_action);
(void)posix_spawn_file_actions_destroy(file_actionsp);
return -1;
}
/*[clinic input]
os.posix_spawn
@ -5124,7 +5229,7 @@ os.posix_spawn
env: object
Dictionary of strings mapping to strings.
file_actions: object = None
FileActions object.
A sequence of file action tuples.
/
Execute the program specified by path in a new process.
@ -5133,22 +5238,22 @@ Execute the program specified by path in a new process.
static PyObject *
os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
PyObject *env, PyObject *file_actions)
/*[clinic end generated code: output=d023521f541c709c input=0ec9f1cfdc890be5]*/
/*[clinic end generated code: output=d023521f541c709c input=a3db1021d33230dc]*/
{
EXECV_CHAR **argvlist = NULL;
EXECV_CHAR **envlist = NULL;
posix_spawn_file_actions_t _file_actions;
posix_spawn_file_actions_t file_actions_buf;
posix_spawn_file_actions_t *file_actionsp = NULL;
Py_ssize_t argc, envc;
PyObject* result = NULL;
PyObject* seq = NULL;
PyObject *result = NULL;
pid_t pid;
int err_code;
/* posix_spawn has three arguments: (path, argv, env), where
argv is a list or tuple of strings and env is a dictionary
argv is a list or tuple of strings and env is a dictionary
like posix.environ. */
if (!PySequence_Check(argv)) {
if (!PyList_Check(argv) && !PyTuple_Check(argv)) {
PyErr_SetString(PyExc_TypeError,
"posix_spawn: argv must be a tuple or list");
goto exit;
@ -5180,139 +5285,35 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
goto exit;
}
pid_t pid;
if (file_actions != NULL && file_actions != Py_None) {
if(posix_spawn_file_actions_init(&_file_actions) != 0){
PyErr_SetString(PyExc_OSError,
"Error initializing file actions");
if (file_actions != Py_None) {
if (parse_file_actions(file_actions, &file_actions_buf)) {
goto exit;
}
file_actionsp = &_file_actions;
seq = PySequence_Fast(file_actions, "file_actions must be a sequence");
if(seq == NULL) {
goto exit;
}
PyObject* file_actions_obj;
PyObject* mode_obj;
for (int i = 0; i < PySequence_Fast_GET_SIZE(seq); ++i) {
file_actions_obj = PySequence_Fast_GET_ITEM(seq, i);
if(!PySequence_Check(file_actions_obj) | !PySequence_Size(file_actions_obj)) {
PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence");
goto exit;
}
mode_obj = PySequence_Fast_GET_ITEM(file_actions_obj, 0);
int mode = PyLong_AsLong(mode_obj);
/* Populate the file_actions object */
switch(mode) {
case POSIX_SPAWN_OPEN:
if(PySequence_Size(file_actions_obj) != 5) {
PyErr_SetString(PyExc_TypeError,"A open file_action object must have 5 elements");
goto exit;
}
long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
if(PyErr_Occurred()) {
goto exit;
}
const char* open_path = PyUnicode_AsUTF8(PySequence_GetItem(file_actions_obj, 2));
if(open_path == NULL) {
goto exit;
}
long open_oflag = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 3));
if(PyErr_Occurred()) {
goto exit;
}
long open_mode = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 4));
if(PyErr_Occurred()) {
goto exit;
}
if(posix_spawn_file_actions_addopen(file_actionsp, open_fd, open_path, open_oflag, open_mode)) {
PyErr_SetString(PyExc_OSError,"Failed to add open file action");
goto exit;
}
break;
case POSIX_SPAWN_CLOSE:
if(PySequence_Size(file_actions_obj) != 2){
PyErr_SetString(PyExc_TypeError,"A close file_action object must have 2 elements");
goto exit;
}
long close_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
if(PyErr_Occurred()) {
goto exit;
}
if(posix_spawn_file_actions_addclose(file_actionsp, close_fd)) {
PyErr_SetString(PyExc_OSError,"Failed to add close file action");
goto exit;
}
break;
case POSIX_SPAWN_DUP2:
if(PySequence_Size(file_actions_obj) != 3){
PyErr_SetString(PyExc_TypeError,"A dup2 file_action object must have 3 elements");
goto exit;
}
long fd1 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
if(PyErr_Occurred()) {
goto exit;
}
long fd2 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 2));
if(PyErr_Occurred()) {
goto exit;
}
if(posix_spawn_file_actions_adddup2(file_actionsp, fd1, fd2)) {
PyErr_SetString(PyExc_OSError,"Failed to add dup2 file action");
goto exit;
}
break;
default:
PyErr_SetString(PyExc_TypeError,"Unknown file_actions identifier");
goto exit;
}
}
file_actionsp = &file_actions_buf;
}
_Py_BEGIN_SUPPRESS_IPH
int err_code = posix_spawn(&pid, path->narrow, file_actionsp, NULL, argvlist, envlist);
err_code = posix_spawn(&pid, path->narrow,
file_actionsp, NULL, argvlist, envlist);
_Py_END_SUPPRESS_IPH
if(err_code) {
PyErr_SetString(PyExc_OSError,"posix_spawn call failed");
if (err_code) {
errno = err_code;
PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, path->object);
goto exit;
}
result = PyLong_FromPid(pid);
exit:
Py_XDECREF(seq);
if(file_actionsp) {
posix_spawn_file_actions_destroy(file_actionsp);
if (file_actionsp) {
(void)posix_spawn_file_actions_destroy(file_actionsp);
}
if (envlist) {
free_string_array(envlist, envc);
}
if (argvlist) {
free_string_array(argvlist, argc);
}
return result;
}
#endif /* HAVE_POSIX_SPAWN */