diff options
| -rw-r--r-- | Doc/library/os.rst | 38 | ||||
| -rw-r--r-- | Lib/test/test_posix.py | 177 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Library/2018-04-01-19-21-04.bpo-20104.-AKcGa.rst | 1 | ||||
| -rw-r--r-- | Modules/clinic/posixmodule.c.h | 4 | ||||
| -rw-r--r-- | Modules/posixmodule.c | 245 |
5 files changed, 313 insertions, 152 deletions
diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 2e81459..4b4e931 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -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 diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 8ada0e3..b94da3f 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -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() diff --git a/Misc/NEWS.d/next/Library/2018-04-01-19-21-04.bpo-20104.-AKcGa.rst b/Misc/NEWS.d/next/Library/2018-04-01-19-21-04.bpo-20104.-AKcGa.rst new file mode 100644 index 0000000..150401d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-04-01-19-21-04.bpo-20104.-AKcGa.rst @@ -0,0 +1 @@ +Improved error handling and fixed a reference leak in :func:`os.posix_spawn()`. diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 4054389..e4bbd08 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -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]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index e672103..4ac6e76 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -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"); - goto exit; - } - - file_actionsp = &_file_actions; - - seq = PySequence_Fast(file_actions, "file_actions must be a sequence"); - if(seq == NULL) { + if (file_actions != Py_None) { + if (parse_file_actions(file_actions, &file_actions_buf)) { 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 */ |