◐ Shell
reader mode source ↗
Skip to content
Closed

bpo-33191: Fix refleaks, style and other issues for os.posix_spawn #6331

Show file tree
Hide file tree
Changes from all commits
File filter
Conversations
Jump to
Diff view
Apply and reload
Show whitespace
Diff view
Apply and reload
11 changes: 6 additions & 5 deletions Lib/test/test_posix.py
View file Edit file Delete file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,17 @@ def test_fexecve(self):

@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))

Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2018-04-01-16-20-05.bpo-33191.hVSbgx.rst
View file Edit file Delete file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
93 changes: 47 additions & 46 deletions Modules/posixmodule.c
View file Edit file Delete file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5142,15 +5142,15 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
Py_ssize_t argc, envc;
PyObject* result = NULL;
PyObject* seq = NULL;


/* posix_spawn has three arguments: (path, argv, env), where
argv is a list or tuple of strings and env is a dictionary
like posix.environ. */

if (!PySequence_Check(argv)) {
PyErr_SetString(PyExc_TypeError,
"posix_spawn: argv must be a tuple or list");
goto exit;
}
argc = PySequence_Size(argv);
Expand Down Expand Up @@ -5181,8 +5181,8 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
}

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;
Expand All @@ -5191,104 +5191,104 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
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;
}
}
}

_Py_BEGIN_SUPPRESS_IPH
int 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");
goto exit;
}
Expand All @@ -5297,11 +5297,12 @@ os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
exit:

Py_XDECREF(seq);

if(file_actionsp) {
posix_spawn_file_actions_destroy(file_actionsp);
}
if (envlist) {
free_string_array(envlist, envc);
}
Toggle all file notes Toggle all file annotations