[Python-checkins] cpython (3.3): Fixes issue #15798: subprocess.Popen() no longer fails if file

[gregory.p.smith]

http://hg.python.org/cpython/rev/07425df887b5
changeset:   87686:07425df887b5
branch:      3.3
parent:      87681:c6bb6f304f75
user:        Gregory P. Smith <greg at krypto.org>
date:        Sun Dec 01 17:27:40 2013 -0800
summary:
  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.

files:
  Lib/test/test_subprocess.py |  21 ++++++++++
  Misc/NEWS                   |   3 +
  Modules/_posixsubprocess.c  |  48 +++++++++++++++++++++---
  3 files changed, 66 insertions(+), 6 deletions(-)


diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1559,6 +1559,27 @@
         # all standard fds closed.
         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):
         # open up some temporary files
         temps = [mkstemp() for i in range(3)]
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -18,6 +18,9 @@
 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
   object.__reduce__() and object.__reduce_ex__().
 
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -723,26 +723,24 @@
 
 PyDoc_STRVAR(subprocess_cloexec_pipe_doc,
 "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 *
 subprocess_cloexec_pipe(PyObject *self, PyObject *noargs)
 {
     int fds[2];
-    int res;
+    int res, saved_errno;
+    long oldflags;
 #ifdef HAVE_PIPE2
     Py_BEGIN_ALLOW_THREADS
     res = pipe2(fds, O_CLOEXEC);
     Py_END_ALLOW_THREADS
     if (res != 0 && errno == ENOSYS)
     {
-        {
 #endif
         /* 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
          * anything without pipe2(). */
-        long oldflags;
-
         res = pipe(fds);
 
         if (res == 0) {
@@ -759,9 +757,47 @@
         if (res == 0)
             res = fcntl(fds[1], F_SETFD, oldflags | FD_CLOEXEC);
 #ifdef HAVE_PIPE2
-        }
     }
 #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)
         return PyErr_SetFromErrno(PyExc_OSError);
     return Py_BuildValue("(ii)", fds[0], fds[1]);

-- 
Repository URL: http://hg.python.org/cpython