packages/python3
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
python3/backport-20104-Improve-error-handling-and-fix-a-reference.patch
hanxinke 046f4dd6a3 python3: expose posix_spawn in os module
2021-05-25 05:10:34 -04:00

544 lines
20 KiB
Diff
Raw Blame History

From bebc98800f4a760d03c3ee9fe49e2943132b47c6 Mon Sep 17 00:00:00 2001
From: Serhiy Storchaka <storchaka@gmail.com>
Date: Tue, 1 May 2018 16:45:04 +0300
Subject: [PATCH] bpo-20104: Improve error handling and fix a reference leak in
os.posix_spawn(). (#6332)
Conflict:NA
Reference:https://github.com/python/cpython/commit/ef347535f289baad22c0601e12a36b2dcd155c3a
Signed-off-by: hanxinke <hanxinke@huawei.com>
---
Lib/test/test_posix.py | 177 +++++++++++--
.../2018-04-01-19-21-04.bpo-20104.-AKcGa.rst | 1 +
Modules/clinic/posixmodule.c.h | 2 +-
Modules/posixmodule.c | 245 +++++++++---------
4 files changed, 285 insertions(+), 140 deletions(-)
create mode 100644 Misc/NEWS.d/next/Library/2018-04-01-19-21-04.bpo-20104.-AKcGa.rst
diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
index c67087c..a22baac 100644
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -192,22 +192,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):
@@ -1519,9 +1503,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 07be916..1371ef6 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__},
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index b5636f5..f10fe2b 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -5138,6 +5138,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
@@ -5148,7 +5253,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.
@@ -5157,22 +5262,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;
@@ -5204,139 +5309,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 */
--
2.23.0
Reference in New Issue View Git Blame Copy Permalink