From 249cdc39fcfb0405ccb2c952f5d8bfce77deb53d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Fran=C3=A7ois=20Natali?= Date: Sun, 25 Aug 2013 18:24:45 +0200 Subject: Issue #18763: subprocess: The file descriptors are now closed after calling the preexec_fn callback, which may open file descriptors. --- Lib/test/test_subprocess.py | 17 +++++++++++++++++ Modules/_posixsubprocess.c | 23 ++++++++++++----------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 0aa4d21..f8cd1de 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1938,6 +1938,23 @@ class POSIXProcessTestCase(BaseTestCase): self.assertRaises(OSError, os.waitpid, pid, 0) self.assertNotIn(ident, [id(o) for o in subprocess._active]) + def test_close_fds_after_preexec(self): + fd_status = support.findfile("fd_status.py", subdir="subprocessdata") + + # this FD is used as dup2() target by preexec_fn, and should be closed + # in the child process + fd = os.dup(1) + self.addCleanup(os.close, fd) + + p = subprocess.Popen([sys.executable, fd_status], + stdout=subprocess.PIPE, close_fds=True, + preexec_fn=lambda: os.dup2(1, fd)) + output, ignored = p.communicate() + + remaining_fds = set(map(int, output.split(b','))) + + self.assertNotIn(fd, remaining_fds) + @unittest.skipUnless(mswindows, "Windows specific tests") class Win32ProcessTestCase(BaseTestCase): diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 07d7096..8d65530 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -412,17 +412,6 @@ child_exec(char *const exec_array[], POSIX_CALL(close(errwrite)); } - if (close_fds) { - int local_max_fd = max_fd; -#if defined(__NetBSD__) - local_max_fd = fcntl(0, F_MAXFD); - if (local_max_fd < 0) - local_max_fd = max_fd; -#endif - /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */ - _close_open_fd_range(3, local_max_fd, py_fds_to_keep); - } - if (cwd) POSIX_CALL(chdir(cwd)); @@ -451,6 +440,18 @@ child_exec(char *const exec_array[], /* Py_DECREF(result); - We're about to exec so why bother? */ } + /* close FDs after executing preexec_fn, which might open FDs */ + if (close_fds) { + int local_max_fd = max_fd; +#if defined(__NetBSD__) + local_max_fd = fcntl(0, F_MAXFD); + if (local_max_fd < 0) + local_max_fd = max_fd; +#endif + /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */ + _close_open_fd_range(3, local_max_fd, py_fds_to_keep); + } + /* This loop matches the Lib/os.py _execvpe()'s PATH search when */ /* given the executable_list generated by Lib/subprocess.py. */ saved_errno = 0; -- cgit v0.12