Commit 1ff8c1cf authored by Gregory P. Smith's avatar Gregory P. Smith

Fixes issue #15798: subprocess.Popen() no longer fails if file

descriptor 0, 1 or 2 is closed.
The errpipe_write fd will always be >= 3.
parent e99a01ab
...@@ -1559,6 +1559,27 @@ class POSIXProcessTestCase(BaseTestCase): ...@@ -1559,6 +1559,27 @@ class POSIXProcessTestCase(BaseTestCase):
# all standard fds closed. # all standard fds closed.
self.check_close_std_fds([0, 1, 2]) self.check_close_std_fds([0, 1, 2])
def test_small_errpipe_write_fd(self):
"""Issue #15798: Popen should work when stdio fds are available."""
new_stdin = os.dup(0)
new_stdout = os.dup(1)
try:
os.close(0)
os.close(1)
# Side test: if errpipe_write fails to have its CLOEXEC
# flag set this should cause the parent to think the exec
# failed. Extremely unlikely: everyone supports CLOEXEC.
subprocess.Popen([
sys.executable, "-c",
"print('AssertionError:0:CLOEXEC failure.')"]).wait()
finally:
# Restore original stdin and stdout
os.dup2(new_stdin, 0)
os.dup2(new_stdout, 1)
os.close(new_stdin)
os.close(new_stdout)
def test_remapping_std_fds(self): def test_remapping_std_fds(self):
# open up some temporary files # open up some temporary files
temps = [mkstemp() for i in range(3)] temps = [mkstemp() for i in range(3)]
......
...@@ -18,6 +18,9 @@ Core and Builtins ...@@ -18,6 +18,9 @@ Core and Builtins
Library Library
------- -------
- Issue #15798: Fixed subprocess.Popen() to no longer fail if file
descriptor 0, 1 or 2 is closed.
- Issue #19088: Fixed incorrect caching of the copyreg module in - Issue #19088: Fixed incorrect caching of the copyreg module in
object.__reduce__() and object.__reduce_ex__(). object.__reduce__() and object.__reduce_ex__().
......
...@@ -723,26 +723,24 @@ Raises: Only on an error in the parent process.\n\ ...@@ -723,26 +723,24 @@ Raises: Only on an error in the parent process.\n\
PyDoc_STRVAR(subprocess_cloexec_pipe_doc, PyDoc_STRVAR(subprocess_cloexec_pipe_doc,
"cloexec_pipe() -> (read_end, write_end)\n\n\ "cloexec_pipe() -> (read_end, write_end)\n\n\
Create a pipe whose ends have the cloexec flag set."); Create a pipe whose ends have the cloexec flag set; write_end will be >= 3.");
static PyObject * static PyObject *
subprocess_cloexec_pipe(PyObject *self, PyObject *noargs) subprocess_cloexec_pipe(PyObject *self, PyObject *noargs)
{ {
int fds[2]; int fds[2];
int res; int res, saved_errno;
long oldflags;
#ifdef HAVE_PIPE2 #ifdef HAVE_PIPE2
Py_BEGIN_ALLOW_THREADS Py_BEGIN_ALLOW_THREADS
res = pipe2(fds, O_CLOEXEC); res = pipe2(fds, O_CLOEXEC);
Py_END_ALLOW_THREADS Py_END_ALLOW_THREADS
if (res != 0 && errno == ENOSYS) if (res != 0 && errno == ENOSYS)
{ {
{
#endif #endif
/* We hold the GIL which offers some protection from other code calling /* We hold the GIL which offers some protection from other code calling
* fork() before the CLOEXEC flags have been set but we can't guarantee * fork() before the CLOEXEC flags have been set but we can't guarantee
* anything without pipe2(). */ * anything without pipe2(). */
long oldflags;
res = pipe(fds); res = pipe(fds);
if (res == 0) { if (res == 0) {
...@@ -760,8 +758,46 @@ subprocess_cloexec_pipe(PyObject *self, PyObject *noargs) ...@@ -760,8 +758,46 @@ subprocess_cloexec_pipe(PyObject *self, PyObject *noargs)
res = fcntl(fds[1], F_SETFD, oldflags | FD_CLOEXEC); res = fcntl(fds[1], F_SETFD, oldflags | FD_CLOEXEC);
#ifdef HAVE_PIPE2 #ifdef HAVE_PIPE2
} }
}
#endif #endif
if (res == 0 && fds[1] < 3) {
/* We always want the write end of the pipe to avoid fds 0, 1 and 2
* as our child may claim those for stdio connections. */
int write_fd = fds[1];
int fds_to_close[3] = {-1, -1, -1};
int fds_to_close_idx = 0;
#ifdef F_DUPFD_CLOEXEC
fds_to_close[fds_to_close_idx++] = write_fd;
write_fd = fcntl(write_fd, F_DUPFD_CLOEXEC, 3);
if (write_fd < 0) /* We don't support F_DUPFD_CLOEXEC / other error */
#endif
{
/* Use dup a few times until we get a desirable fd. */
for (; fds_to_close_idx < 3; ++fds_to_close_idx) {
fds_to_close[fds_to_close_idx] = write_fd;
write_fd = dup(write_fd);
if (write_fd >= 3)
break;
/* We may dup a few extra times if it returns an error but
* that is okay. Repeat calls should return the same error. */
}
if (write_fd < 0) res = write_fd;
if (res == 0) {
oldflags = fcntl(write_fd, F_GETFD, 0);
if (oldflags < 0) res = oldflags;
if (res == 0)
res = fcntl(write_fd, F_SETFD, oldflags | FD_CLOEXEC);
}
}
saved_errno = errno;
/* Close fds we tried for the write end that were too low. */
for (fds_to_close_idx=0; fds_to_close_idx < 3; ++fds_to_close_idx) {
int temp_fd = fds_to_close[fds_to_close_idx];
while (temp_fd >= 0 && close(temp_fd) < 0 && errno == EINTR);
}
errno = saved_errno; /* report dup or fcntl errors, not close. */
fds[1] = write_fd;
} /* end if write fd was too small */
if (res != 0) if (res != 0)
return PyErr_SetFromErrno(PyExc_OSError); return PyErr_SetFromErrno(PyExc_OSError);
return Py_BuildValue("(ii)", fds[0], fds[1]); return Py_BuildValue("(ii)", fds[0], fds[1]);
......
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