diff options
| -rw-r--r-- | Lib/subprocess.py | 3 | ||||
| -rw-r--r-- | Lib/test/test_subprocess.py | 38 | ||||
| -rw-r--r-- | Misc/NEWS | 3 |
3 files changed, 41 insertions, 3 deletions
diff --git a/Lib/subprocess.py b/Lib/subprocess.py index dad7b16..d3d90ca 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1419,9 +1419,6 @@ class Popen(object): child_exception_type = getattr( builtins, exception_name.decode('ascii'), RuntimeError) - for fd in (p2cwrite, c2pread, errread): - if fd != -1: - os.close(fd) err_msg = err_msg.decode(errors="surrogatepass") if issubclass(child_exception_type, OSError) and hex_errno: errno_num = int(hex_errno, 16) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 2d77554..da7045c 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1189,6 +1189,44 @@ class POSIXProcessTestCase(BaseTestCase): self.fail("Exception raised by preexec_fn did not make it " "to the parent process.") + @unittest.skipIf(not os.path.exists("/dev/zero"), "/dev/zero required.") + def test_preexec_errpipe_does_not_double_close_pipes(self): + """Issue16140: Don't double close pipes on preexec error.""" + class SafeConstructorPopen(subprocess.Popen): + def __init__(self): + pass # Do nothing so we can modify the instance for testing. + def RealPopen(self, *args, **kwargs): + subprocess.Popen.__init__(self, *args, **kwargs) + def raise_it(): + raise RuntimeError("force the _execute_child() errpipe_data path.") + + p = SafeConstructorPopen() + + def _test_fds_execute_child_wrapper(*args, **kwargs): + try: + subprocess.Popen._execute_child(p, *args, **kwargs) + finally: + # Open a bunch of file descriptors and verify that + # none of them are the same as the ones the Popen + # instance is using for stdin/stdout/stderr. + devzero_fds = [os.open("/dev/zero", os.O_RDONLY) + for _ in range(8)] + try: + for fd in devzero_fds: + self.assertNotIn(fd, ( + p.stdin.fileno(), p.stdout.fileno(), + p.stderr.fileno()), + msg="At least one fd was closed early.") + finally: + map(os.close, devzero_fds) + + p._execute_child = _test_fds_execute_child_wrapper + + with self.assertRaises(RuntimeError): + p.RealPopen([sys.executable, "-c", "pass"], + stdin=subprocess.PIPE, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, preexec_fn=raise_it) + def test_preexec_gc_module_failure(self): # This tests the code that disables garbage collection if the child # process will execute any Python. @@ -83,6 +83,9 @@ Core and Builtins Library ------- +- Issue #16140: The subprocess module no longer double closes its child + subprocess.PIPE parent file descriptors on child error prior to exec(). + - Remove a bare print to stdout from the subprocess module that could have happened if the child process wrote garbage to its pre-exec error pipe. |