bpo-32270: Don't close stdin/out/err in pass_fds (GH-6242) (GH-9148)
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.
(cherry picked from commit ce34410b8b67f49d8275c05d51b3ead50cf97f48)
Co-authored-by: Gregory P. Smith <greg@krypto.org> [Google]
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 0150fcb..aeb10f9 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -422,10 +422,20 @@
/* 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 @@
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));