Commit d4cc7bf9 authored by Gregory P. Smith's avatar Gregory P. Smith

issue6559: Adds a pass_fds parameter to subprocess.Popen that allows the caller

to list exactly which file descriptors should be kept open.
parent 1acb746d
...@@ -608,7 +608,8 @@ class Popen(object): ...@@ -608,7 +608,8 @@ class Popen(object):
preexec_fn=None, close_fds=None, shell=False, preexec_fn=None, close_fds=None, shell=False,
cwd=None, env=None, universal_newlines=False, cwd=None, env=None, universal_newlines=False,
startupinfo=None, creationflags=0, startupinfo=None, creationflags=0,
restore_signals=True, start_new_session=False): restore_signals=True, start_new_session=False,
pass_fds=()):
"""Create new Popen instance.""" """Create new Popen instance."""
_cleanup() _cleanup()
...@@ -644,6 +645,9 @@ class Popen(object): ...@@ -644,6 +645,9 @@ class Popen(object):
raise ValueError("creationflags is only supported on Windows " raise ValueError("creationflags is only supported on Windows "
"platforms") "platforms")
if pass_fds and not close_fds:
raise ValueError("pass_fds requires close_fds=True.")
self.stdin = None self.stdin = None
self.stdout = None self.stdout = None
self.stderr = None self.stderr = None
...@@ -671,7 +675,7 @@ class Popen(object): ...@@ -671,7 +675,7 @@ class Popen(object):
errread, errwrite) = self._get_handles(stdin, stdout, stderr) errread, errwrite) = self._get_handles(stdin, stdout, stderr)
self._execute_child(args, executable, preexec_fn, close_fds, self._execute_child(args, executable, preexec_fn, close_fds,
cwd, env, universal_newlines, pass_fds, cwd, env, universal_newlines,
startupinfo, creationflags, shell, startupinfo, creationflags, shell,
p2cread, p2cwrite, p2cread, p2cwrite,
c2pread, c2pwrite, c2pread, c2pwrite,
...@@ -848,7 +852,7 @@ class Popen(object): ...@@ -848,7 +852,7 @@ class Popen(object):
def _execute_child(self, args, executable, preexec_fn, close_fds, def _execute_child(self, args, executable, preexec_fn, close_fds,
cwd, env, universal_newlines, pass_fds, cwd, env, universal_newlines,
startupinfo, creationflags, shell, startupinfo, creationflags, shell,
p2cread, p2cwrite, p2cread, p2cwrite,
c2pread, c2pwrite, c2pread, c2pwrite,
...@@ -856,6 +860,8 @@ class Popen(object): ...@@ -856,6 +860,8 @@ class Popen(object):
unused_restore_signals, unused_start_new_session): unused_restore_signals, unused_start_new_session):
"""Execute program (MS Windows version)""" """Execute program (MS Windows version)"""
assert not pass_fds, "pass_fds not yet supported on Windows"
if not isinstance(args, str): if not isinstance(args, str):
args = list2cmdline(args) args = list2cmdline(args)
...@@ -1075,8 +1081,19 @@ class Popen(object): ...@@ -1075,8 +1081,19 @@ class Popen(object):
os.closerange(but + 1, MAXFD) os.closerange(but + 1, MAXFD)
def _close_all_but_a_sorted_few_fds(self, fds_to_keep):
# precondition: fds_to_keep must be sorted and unique
start_fd = 3
for fd in fds_to_keep:
if fd > start_fd:
os.closerange(start_fd, fd)
start_fd = fd + 1
if start_fd <= MAXFD:
os.closerange(start_fd, MAXFD)
def _execute_child(self, args, executable, preexec_fn, close_fds, def _execute_child(self, args, executable, preexec_fn, close_fds,
cwd, env, universal_newlines, pass_fds, cwd, env, universal_newlines,
startupinfo, creationflags, shell, startupinfo, creationflags, shell,
p2cread, p2cwrite, p2cread, p2cwrite,
c2pread, c2pwrite, c2pread, c2pwrite,
...@@ -1124,9 +1141,11 @@ class Popen(object): ...@@ -1124,9 +1141,11 @@ class Popen(object):
executable_list = tuple( executable_list = tuple(
os.path.join(os.fsencode(dir), executable) os.path.join(os.fsencode(dir), executable)
for dir in os.get_exec_path(env)) for dir in os.get_exec_path(env))
fds_to_keep = set(pass_fds)
fds_to_keep.add(errpipe_write)
self.pid = _posixsubprocess.fork_exec( self.pid = _posixsubprocess.fork_exec(
args, executable_list, args, executable_list,
close_fds, cwd, env_list, close_fds, sorted(fds_to_keep), cwd, env_list,
p2cread, p2cwrite, c2pread, c2pwrite, p2cread, p2cwrite, c2pread, c2pwrite,
errread, errwrite, errread, errwrite,
errpipe_read, errpipe_write, errpipe_read, errpipe_write,
...@@ -1180,7 +1199,14 @@ class Popen(object): ...@@ -1180,7 +1199,14 @@ class Popen(object):
# Close all other fds, if asked for # Close all other fds, if asked for
if close_fds: if close_fds:
self._close_fds(but=errpipe_write) if pass_fds:
fds_to_keep = set(pass_fds)
fds_to_keep.add(errpipe_write)
self._close_all_but_a_sorted_few_fds(
sorted(fds_to_keep))
else:
self._close_fds(but=errpipe_write)
if cwd is not None: if cwd is not None:
os.chdir(cwd) os.chdir(cwd)
......
...@@ -42,7 +42,8 @@ static void child_exec(char *const exec_array[], ...@@ -42,7 +42,8 @@ static void child_exec(char *const exec_array[],
int errread, int errwrite, int errread, int errwrite,
int errpipe_read, int errpipe_write, int errpipe_read, int errpipe_write,
int close_fds, int restore_signals, int close_fds, int restore_signals,
int call_setsid, int call_setsid, Py_ssize_t num_fds_to_keep,
PyObject *py_fds_to_keep,
PyObject *preexec_fn, PyObject *preexec_fn,
PyObject *preexec_fn_args_tuple) PyObject *preexec_fn_args_tuple)
{ {
...@@ -91,11 +92,28 @@ static void child_exec(char *const exec_array[], ...@@ -91,11 +92,28 @@ static void child_exec(char *const exec_array[],
/* close() is intentionally not checked for errors here as we are closing */ /* close() is intentionally not checked for errors here as we are closing */
/* a large range of fds, some of which may be invalid. */ /* a large range of fds, some of which may be invalid. */
if (close_fds) { if (close_fds) {
for (fd_num = 3; fd_num < errpipe_write; ++fd_num) { Py_ssize_t keep_seq_idx;
close(fd_num); int start_fd = 3;
for (keep_seq_idx = 0; keep_seq_idx < num_fds_to_keep; ++keep_seq_idx) {
PyObject* py_keep_fd = PySequence_Fast_GET_ITEM(py_fds_to_keep,
keep_seq_idx);
int keep_fd = PyLong_AsLong(py_keep_fd);
if (keep_fd < 0) { /* Negative number, overflow or not a Long. */
err_msg = "bad value in fds_to_keep.";
errno = 0; /* We don't want to report an OSError. */
goto error;
}
if (keep_fd <= start_fd)
continue;
for (fd_num = start_fd; fd_num < keep_fd; ++fd_num) {
close(fd_num);
}
start_fd = keep_fd + 1;
} }
for (fd_num = errpipe_write+1; fd_num < max_fd; ++fd_num) { if (start_fd <= max_fd) {
close(fd_num); for (fd_num = start_fd; fd_num < max_fd; ++fd_num) {
close(fd_num);
}
} }
} }
...@@ -170,7 +188,7 @@ static PyObject * ...@@ -170,7 +188,7 @@ static PyObject *
subprocess_fork_exec(PyObject* self, PyObject *args) subprocess_fork_exec(PyObject* self, PyObject *args)
{ {
PyObject *gc_module = NULL; PyObject *gc_module = NULL;
PyObject *executable_list, *py_close_fds; PyObject *executable_list, *py_close_fds, *py_fds_to_keep;
PyObject *env_list, *preexec_fn; PyObject *env_list, *preexec_fn;
PyObject *process_args, *converted_args = NULL, *fast_args = NULL; PyObject *process_args, *converted_args = NULL, *fast_args = NULL;
PyObject *preexec_fn_args_tuple = NULL; PyObject *preexec_fn_args_tuple = NULL;
...@@ -182,11 +200,11 @@ subprocess_fork_exec(PyObject* self, PyObject *args) ...@@ -182,11 +200,11 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
pid_t pid; pid_t pid;
int need_to_reenable_gc = 0; int need_to_reenable_gc = 0;
char *const *exec_array, *const *argv = NULL, *const *envp = NULL; char *const *exec_array, *const *argv = NULL, *const *envp = NULL;
Py_ssize_t arg_num; Py_ssize_t arg_num, num_fds_to_keep;
if (!PyArg_ParseTuple( if (!PyArg_ParseTuple(
args, "OOOOOiiiiiiiiiiO:fork_exec", args, "OOOOOOiiiiiiiiiiO:fork_exec",
&process_args, &executable_list, &py_close_fds, &process_args, &executable_list, &py_close_fds, &py_fds_to_keep,
&cwd_obj, &env_list, &cwd_obj, &env_list,
&p2cread, &p2cwrite, &c2pread, &c2pwrite, &p2cread, &p2cwrite, &c2pread, &c2pwrite,
&errread, &errwrite, &errpipe_read, &errpipe_write, &errread, &errwrite, &errpipe_read, &errpipe_write,
...@@ -198,6 +216,11 @@ subprocess_fork_exec(PyObject* self, PyObject *args) ...@@ -198,6 +216,11 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
PyErr_SetString(PyExc_ValueError, "errpipe_write must be >= 3"); PyErr_SetString(PyExc_ValueError, "errpipe_write must be >= 3");
return NULL; return NULL;
} }
num_fds_to_keep = PySequence_Length(py_fds_to_keep);
if (num_fds_to_keep < 0) {
PyErr_SetString(PyExc_ValueError, "bad fds_to_keep");
return NULL;
}
/* We need to call gc.disable() when we'll be calling preexec_fn */ /* We need to call gc.disable() when we'll be calling preexec_fn */
if (preexec_fn != Py_None) { if (preexec_fn != Py_None) {
...@@ -298,6 +321,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args) ...@@ -298,6 +321,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
p2cread, p2cwrite, c2pread, c2pwrite, p2cread, p2cwrite, c2pread, c2pwrite,
errread, errwrite, errpipe_read, errpipe_write, errread, errwrite, errpipe_read, errpipe_write,
close_fds, restore_signals, call_setsid, close_fds, restore_signals, call_setsid,
num_fds_to_keep, py_fds_to_keep,
preexec_fn, preexec_fn_args_tuple); preexec_fn, preexec_fn_args_tuple);
_exit(255); _exit(255);
return NULL; /* Dead code to avoid a potential compiler warning. */ return NULL; /* Dead code to avoid a potential compiler warning. */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment