diff --git a/Lib/subprocess.py b/Lib/subprocess.py index a659087ab6c..c3a278836c1 100644 --- 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 f5a70a38734..77f1ba31551 100644 --- 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 4742b9b1c70..0acbafd600b 100644 --- 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 fa1975cce9c..8d655306ff6 100644 --- 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;