Skip to content

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Sep 11, 2018

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 izbyshev@b89b52f

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 ce34410)

Co-authored-by: Gregory P. Smith greg@krypto.org

https://bugs.python.org/issue32270

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 izbyshev@b89b52f

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 ce34410)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
@miss-islington
Copy link
Contributor Author

@gpshead: Backport status check is done, and it's a failure ❌ .

1 similar comment
@miss-islington
Copy link
Contributor Author

@gpshead: Backport status check is done, and it's a failure ❌ .

@gpshead
Copy link
Member

gpshead commented Sep 11, 2018

Ignoring non-required appveyor infrastructure failure and merging.

@gpshead gpshead merged commit 2173bb8 into python:3.6 Sep 11, 2018
@miss-islington miss-islington deleted the backport-ce34410-3.6 branch September 11, 2018 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants