Undo supposed fix for Issue #15798 until I understand why this is

causing test_multiprocessing_forkserver and test_multiprocessing_spawn
failures on head (3.4).

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index a659087ab6c3b92eb47464c4584c1fb63f863de5..c3a278836c19121319363f1c896e528b48968202 100644 (file)
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1368,10 +1368,7 @@ class Popen(object):
                         executable_list = tuple(
                             os.path.join(os.fsencode(dir), executable)
                             for dir in os.get_exec_path(env))
-                    # Never close stdin, stdout and stderr for the child.
-                    fds_to_keep = {0,1,2}
-                    fds_to_keep.update(pass_fds)
-                    # Our child uses this one to signal error before exec().
+                    fds_to_keep = set(pass_fds)
                     fds_to_keep.add(errpipe_write)
                     self.pid = _posixsubprocess.fork_exec(
                             args, executable_list,

diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index f5a70a3873490c562d05b12706218f19c1f668b9..77f1ba31551190e31428aea97e685d6bc36cd01d 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1559,27 +1559,6 @@ class POSIXProcessTestCase(BaseTestCase):
         # 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
index 4742b9b1c70ca38ebf13c4c1e8286a7070d6d267..0acbafd600b02072359f89a4b7b0685532961ddf 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -18,9 +18,6 @@ Core and Builtins
 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
index fa1975cce9c5a13b4e2f382465026f3199c60add..8d655306ff6790fa965b6e84d14f69ff3133565d 100644 (file)
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -449,7 +449,7 @@ child_exec(char *const exec_array[],
             local_max_fd = max_fd;
 #endif
         /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */
-        _close_open_fd_range(0, local_max_fd, py_fds_to_keep);
+        _close_open_fd_range(3, local_max_fd, py_fds_to_keep);
     }
 
     /* This loop matches the Lib/os.py _execvpe()'s PATH search when */
@@ -526,6 +526,10 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
             &restore_signals, &call_setsid, &preexec_fn))
         return NULL;
 
+    if (close_fds && errpipe_write < 3) {  /* precondition */
+        PyErr_SetString(PyExc_ValueError, "errpipe_write must be >= 3");
+        return NULL;
+    }
     if (PySequence_Length(py_fds_to_keep) < 0) {
         PyErr_SetString(PyExc_ValueError, "cannot get length of fds_to_keep");
         return NULL;