Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions Lib/test/test_posix.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,23 @@ def test_fexecve(self):
finally:
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):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Expose posix_spawn as a low level API in the os module.
54 changes: 53 additions & 1 deletion Modules/clinic/posixmodule.c.h
Original file line number Diff line number Diff line change
Expand Up @@ -1721,6 +1721,54 @@ os_execve(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k

#endif /* defined(HAVE_EXECV) */

#if defined(HAVE_POSIX_SPAWN)

PyDoc_STRVAR(os_posix_spawn__doc__,
"posix_spawn($module, path, argv, env, file_actions=None, /)\n"
"--\n"
"\n"
"Execute the program specified by path in a new process.\n"
"\n"
" path\n"
" Path of executable file.\n"
" argv\n"
" Tuple or list of strings.\n"
" env\n"
" Dictionary of strings mapping to strings.\n"
" file_actions\n"
" FileActions object.");

#define OS_POSIX_SPAWN_METHODDEF \
{"posix_spawn", (PyCFunction)os_posix_spawn, METH_FASTCALL, os_posix_spawn__doc__},

static PyObject *
os_posix_spawn_impl(PyObject *module, path_t *path, PyObject *argv,
PyObject *env, PyObject *file_actions);

static PyObject *
os_posix_spawn(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
{
PyObject *return_value = NULL;
path_t path = PATH_T_INITIALIZE("posix_spawn", "path", 0, 0);
PyObject *argv;
PyObject *env;
PyObject *file_actions = Py_None;

if (!_PyArg_ParseStack(args, nargs, "O&OO|O:posix_spawn",
path_converter, &path, &argv, &env, &file_actions)) {
goto exit;
}
return_value = os_posix_spawn_impl(module, &path, argv, env, file_actions);

exit:
/* Cleanup for path */
path_cleanup(&path);

return return_value;
}

#endif /* defined(HAVE_POSIX_SPAWN) */

#if (defined(HAVE_SPAWNV) || defined(HAVE_WSPAWNV))

PyDoc_STRVAR(os_spawnv__doc__,
Expand Down Expand Up @@ -6137,6 +6185,10 @@ os_getrandom(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject
#define OS_EXECVE_METHODDEF
#endif /* !defined(OS_EXECVE_METHODDEF) */

#ifndef OS_POSIX_SPAWN_METHODDEF
#define OS_POSIX_SPAWN_METHODDEF
#endif /* !defined(OS_POSIX_SPAWN_METHODDEF) */

#ifndef OS_SPAWNV_METHODDEF
#define OS_SPAWNV_METHODDEF
#endif /* !defined(OS_SPAWNV_METHODDEF) */
Expand Down Expand Up @@ -6528,4 +6580,4 @@ os_getrandom(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject
#ifndef OS_GETRANDOM_METHODDEF
#define OS_GETRANDOM_METHODDEF
#endif /* !defined(OS_GETRANDOM_METHODDEF) */
/*[clinic end generated code: output=06ace805893aa10c input=a9049054013a1b77]*/
/*[clinic end generated code: output=8e5d4a01257b6292 input=a9049054013a1b77]*/
202 changes: 201 additions & 1 deletion Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ corresponding Unix manual entries for more information on calls.");
#else
/* Unix functions that the configure script doesn't check for */
#define HAVE_EXECV 1
#define HAVE_POSIX_SPAWN 1
#define HAVE_FORK 1
#if defined(__USLC__) && defined(__SCO_VERSION__) /* SCO UDK Compiler */
#define HAVE_FORK1 1
Expand Down Expand Up @@ -246,6 +247,10 @@ extern int lstat(const char *, struct stat *);

#endif /* !_MSC_VER */

#ifdef HAVE_POSIX_SPAWN
#include <spawn.h>
#endif

#ifdef HAVE_UTIME_H
#include <utime.h>
#endif /* HAVE_UTIME_H */
Expand Down Expand Up @@ -5097,6 +5102,194 @@ os_execve_impl(PyObject *module, path_t *path, PyObject *argv, PyObject *env)

#endif /* HAVE_EXECV */

#ifdef HAVE_POSIX_SPAWN

enum posix_spawn_file_actions_identifier {
POSIX_SPAWN_OPEN,
POSIX_SPAWN_CLOSE,
POSIX_SPAWN_DUP2
};

/*[clinic input]

os.posix_spawn
path: path_t
Path of executable file.
argv: object
Tuple or list of strings.
env: object
Dictionary of strings mapping to strings.
file_actions: object = None
FileActions object.
/

Execute the program specified by path in a new process.
[clinic start generated code]*/

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]*/
{
EXECV_CHAR **argvlist = NULL;
EXECV_CHAR **envlist;
Py_ssize_t argc, envc;

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad indentation.

like posix.environ. */

if (!PySequence_Check(argv)){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not (!PyList_Check(argv) && !PyTuple_Check(argv)) as in other places?

PyErr_SetString(PyExc_TypeError,
"posix_spawn: argv must be a tuple or list");
goto fail;
}
argc = PySequence_Size(argv);
if (argc < 1) {
PyErr_SetString(PyExc_ValueError, "posix_spawn: argv must not be empty");
return NULL;
}

if (!PyMapping_Check(env)) {
PyErr_SetString(PyExc_TypeError,
"posix_spawn: environment must be a mapping object");
goto fail;
}

argvlist = parse_arglist(argv, &argc);
if (argvlist == NULL) {
goto fail;
}
if (!argvlist[0][0]) {
PyErr_SetString(PyExc_ValueError,
"posix_spawn: argv first element cannot be empty");
goto fail;
}

envlist = parse_envlist(env, &envc);
if (envlist == NULL)
goto fail;

pid_t pid;
posix_spawn_file_actions_t *file_actionsp = NULL;
if (file_actions != NULL && file_actions != Py_None){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_actions is never NULL.

posix_spawn_file_actions_t _file_actions;
if(posix_spawn_file_actions_init(&_file_actions) != 0){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't conform PEP 7. Needed spaces after "if" and before "{".

PyErr_SetString(PyExc_TypeError,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't errno be included in the exception?

"Error initializing file actions");
goto fail;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too much empty lines.


file_actionsp = &_file_actions;


PyObject* seq = PySequence_Fast(file_actions, "file_actions must be a sequence");
if(seq == NULL){
goto fail;
}
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)){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PySequence_Size() can be called before PySequence_Check().

Use PySequence_Fast_GET_SIZE() instead of PySequence_Check().

PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence");
goto fail;
}


mode_obj = PySequence_Fast_GET_ITEM(file_actions_obj, 0);
int mode = PyLong_AsLong(mode_obj);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be checked for error.


/* Populate the file_actions object */

switch(mode) {

case POSIX_SPAWN_OPEN:
if(PySequence_Size(file_actions_obj) != 5){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to check for errors in all of these cases: The PyXXX_AsYYY APIs can cause TypeError or OverflowError. PyLong_As APIs return -1 on error, use PyErr_Occurred() to disambiguate. Check for NULL from PyUnicode_As.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in cb440d5

PyErr_SetString(PyExc_TypeError,"A open file_action object must have 5 elements");
goto fail;
}

long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyLong_AsLong() can call arbitrary Python code. This can cause changing the size of file_actions_obj. Following PySequence_GetItem() can fail.

I would require file_actions_obj to be a tuple and use PyArg_ParseTuple() for parsing it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open_fd should be of type int. Use _PyLong_AsInt().

if(PyErr_Occurred()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use idiomatic code:

if (open_fd == -1 && PyErr_Occurred()) {

goto fail;
}
const char* open_path = PyUnicode_AsUTF8(PySequence_GetItem(file_actions_obj, 2));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use filesystem encoding. PyUnicode_FSDecoder() or like.

if(open_path == NULL){
goto fail;
}
long open_oflag = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 3));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be int.

if(PyErr_Occurred()) {
goto fail;
}
long open_mode = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 4));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check integer overflow when cast to mode_t.

if(PyErr_Occurred()) {
goto fail;
}
posix_spawn_file_actions_addopen(file_actionsp, open_fd, open_path, open_oflag, open_mode);
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 fail;
}

long close_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be int.

if(PyErr_Occurred()) {
goto fail;
}
posix_spawn_file_actions_addclose(file_actionsp, close_fd);
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 fail;
}

long fd1 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be int.

if(PyErr_Occurred()) {
goto fail;
}
long fd2 = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 2));
if(PyErr_Occurred()) {
goto fail;
}
posix_spawn_file_actions_adddup2(file_actionsp, fd1, fd2);
break;

default:
PyErr_SetString(PyExc_TypeError,"Unknown file_actions identifier");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't ValueError more appropriate exception type?

goto fail;
}
}
Py_DECREF(seq);
}

_Py_BEGIN_SUPPRESS_IPH
posix_spawn(&pid, path->narrow, file_actionsp, NULL, argvlist, envlist);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result of the call is not checked.

return PyLong_FromPid(pid);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return before _Py_END_SUPPRESS_IPH? This looks like a bug.

_Py_END_SUPPRESS_IPH

path_error(path);

free_string_array(envlist, envc);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it leaked in error case?


fail:

if (argvlist) {
free_string_array(argvlist, argc);
}
return NULL;


}
#endif /* HAVE_POSIX_SPAWN */


#if defined(HAVE_SPAWNV) || defined(HAVE_WSPAWNV)
/*[clinic input]
Expand Down Expand Up @@ -5189,7 +5382,6 @@ os_spawnv_impl(PyObject *module, int mode, path_t *path, PyObject *argv)
return Py_BuildValue(_Py_PARSE_INTPTR, spawnval);
}


/*[clinic input]
os.spawnve

Expand Down Expand Up @@ -12610,6 +12802,7 @@ static PyMethodDef posix_methods[] = {
OS_NICE_METHODDEF
OS_GETPRIORITY_METHODDEF
OS_SETPRIORITY_METHODDEF
OS_POSIX_SPAWN_METHODDEF
#ifdef HAVE_READLINK
{"readlink", (PyCFunction)posix_readlink,
METH_VARARGS | METH_KEYWORDS,
Expand Down Expand Up @@ -13164,6 +13357,13 @@ all_ins(PyObject *m)
if (PyModule_AddIntConstant(m, "RWF_NOWAIT", RWF_NOWAIT)) return -1;
#endif

/* constants for posix_spawn */
#ifdef HAVE_POSIX_SPAWN
if (PyModule_AddIntConstant(m, "POSIX_SPAWN_OPEN", POSIX_SPAWN_OPEN)) return -1;
if (PyModule_AddIntConstant(m, "POSIX_SPAWN_CLOSE", POSIX_SPAWN_CLOSE)) return -1;
if (PyModule_AddIntConstant(m, "POSIX_SPAWN_DUP2", POSIX_SPAWN_DUP2)) return -1;
#endif

#ifdef HAVE_SPAWNV
if (PyModule_AddIntConstant(m, "P_WAIT", _P_WAIT)) return -1;
if (PyModule_AddIntConstant(m, "P_NOWAIT", _P_NOWAIT)) return -1;
Expand Down
2 changes: 1 addition & 1 deletion configure
Original file line number Diff line number Diff line change
Expand Up @@ -11197,7 +11197,7 @@ for ac_func in alarm accept4 setitimer getitimer bind_textdomain_codeset chown \
initgroups kill killpg lchmod lchown lockf linkat lstat lutimes mmap \
memrchr mbrtowc mkdirat mkfifo \
mkfifoat mknod mknodat mktime mremap nice openat pathconf pause pipe2 plock poll \
posix_fallocate posix_fadvise pread preadv preadv2 \
posix_fallocate posix_fadvise posix_spawn pread preadv preadv2 \
pthread_init pthread_kill putenv pwrite pwritev pwritev2 readlink readlinkat readv realpath renameat \
sem_open sem_timedwait sem_getvalue sem_unlink sendfile setegid seteuid \
setgid sethostname \
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -3431,7 +3431,7 @@ AC_CHECK_FUNCS(alarm accept4 setitimer getitimer bind_textdomain_codeset chown \
initgroups kill killpg lchmod lchown lockf linkat lstat lutimes mmap \
memrchr mbrtowc mkdirat mkfifo \
mkfifoat mknod mknodat mktime mremap nice openat pathconf pause pipe2 plock poll \
posix_fallocate posix_fadvise pread preadv preadv2 \
posix_fallocate posix_fadvise posix_spawn pread preadv preadv2 \
pthread_init pthread_kill putenv pwrite pwritev pwritev2 readlink readlinkat readv realpath renameat \
sem_open sem_timedwait sem_getvalue sem_unlink sendfile setegid seteuid \
setgid sethostname \
Expand Down
3 changes: 3 additions & 0 deletions pyconfig.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,9 @@
/* Define to 1 if you have the `posix_fallocate' function. */
#undef HAVE_POSIX_FALLOCATE

/* Define to 1 if you have the `posix_spawn' function. */
#undef HAVE_POSIX_SPAWN

/* Define to 1 if you have the `pread' function. */
#undef HAVE_PREAD

Expand Down