From ce34410b8b67f49d8275c05d51b3ead50cf97f48 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 10 Sep 2018 17:46:22 -0700 Subject: bpo-32270: Don't close stdin/out/err in pass_fds (GH-6242) When subprocess.Popen() stdin= stdout= or stderr= handles are specified and appear in pass_fds=, don't close the original fds after dup'ing them. This implementation and unittest primarily came from @izbyshev (see the PR) See also https://github.com/izbyshev/cpython/commit/b89b52f28490b69142d5c061604b3a3989cec66c This also removes the old manual p2cread, c2pwrite, and errwrite closing logic as inheritable flags and _close_open_fds takes care of that properly today without special treatment. This code is within child_exec() where it is the only thread so there is no race condition between the dup and _Py_set_inheritable_async_safe call. --- Modules/_posixsubprocess.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'Modules/_posixsubprocess.c') diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 0150fcb097..aeb10f9ecf 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -422,10 +422,20 @@ child_exec(char *const exec_array[], /* When duping fds, if there arises a situation where one of the fds is either 0, 1 or 2, it is possible that it is overwritten (#12607). */ - if (c2pwrite == 0) + if (c2pwrite == 0) { POSIX_CALL(c2pwrite = dup(c2pwrite)); - while (errwrite == 0 || errwrite == 1) + /* issue32270 */ + if (_Py_set_inheritable_async_safe(c2pwrite, 0, NULL) < 0) { + goto error; + } + } + while (errwrite == 0 || errwrite == 1) { POSIX_CALL(errwrite = dup(errwrite)); + /* issue32270 */ + if (_Py_set_inheritable_async_safe(errwrite, 0, NULL) < 0) { + goto error; + } + } /* Dup fds for child. dup2() removes the CLOEXEC flag but we must do it ourselves if dup2() @@ -451,14 +461,8 @@ child_exec(char *const exec_array[], else if (errwrite != -1) POSIX_CALL(dup2(errwrite, 2)); /* stderr */ - /* Close pipe fds. Make sure we don't close the same fd more than */ - /* once, or standard fds. */ - if (p2cread > 2) - POSIX_CALL(close(p2cread)); - if (c2pwrite > 2 && c2pwrite != p2cread) - POSIX_CALL(close(c2pwrite)); - if (errwrite != c2pwrite && errwrite != p2cread && errwrite > 2) - POSIX_CALL(close(errwrite)); + /* We no longer manually close p2cread, c2pwrite, and errwrite here as + * _close_open_fds takes care when it is not already non-inheritable. */ if (cwd) POSIX_CALL(chdir(cwd)); -- cgit v1.2.1