[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

Thank you, Gregory!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset 2173bb818c6c726d831b106ed0d3fad7825905dc by Gregory P. Smith 
(Miss Islington (bot)) in branch '3.6':
bpo-32270: Don't close stdin/out/err in pass_fds (GH-6242) (GH-9149)
https://github.com/python/cpython/commit/2173bb818c6c726d831b106ed0d3fad7825905dc


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset 9c4a63fc17efea31ad41f90d28825be37469e0e2 by Gregory P. Smith 
(Miss Islington (bot)) in branch '3.7':
bpo-32270: Don't close stdin/out/err in pass_fds (GH-6242) (GH-9148)
https://github.com/python/cpython/commit/9c4a63fc17efea31ad41f90d28825be37469e0e2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[miss-islington]

Change by miss-islington :


--
pull_requests: +8595

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[miss-islington]

Change by miss-islington :


--
pull_requests: +8596

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset ce34410b8b67f49d8275c05d51b3ead50cf97f48 by Gregory P. Smith in 
branch 'master':
bpo-32270: Don't close stdin/out/err in pass_fds (GH-6242)
https://github.com/python/cpython/commit/ce34410b8b67f49d8275c05d51b3ead50cf97f48


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

An alternative fix is here: 
https://github.com/izbyshev/cpython/commit/b89b52f28490b69142d5c061604b3a3989cec66c

Feel free to use the test if you don't like the approach :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Gregory P. Smith]

Change by Gregory P. Smith :


--
keywords: +patch
pull_requests: +5977
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

Actually I've started to work on this with #32844, but got no feedback. This 
issue may of course be fixed independently, but the problems with descriptor 
ownership for fds <= 2 will remain unless all ownership problems are fixed at 
once.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Gregory P. Smith]

Gregory P. Smith  added the comment:

This bug stems from:

 https://github.com/python/cpython/blob/master/Modules/_posixsubprocess.c#L454

When stdout=fd is passed, that results in c2pwrite=fd.  The stdin/stdout/stderr 
pipe fds are closed when > 2 without checking to see if they are listed in 
pass_fds.

https://github.com/python/cpython/blob/master/Lib/subprocess.py#L1306 maps the 
following:
 stdin -> p2cread
 stdout -> c2pwrite
 stderr -> errwrite

--
assignee:  -> gregory.p.smith
versions: +Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Martin Panter]

Change by Martin Panter :


--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Antoine Pitrou]

Antoine Pitrou  added the comment:

> Regarding fixing (1), I'm worrying about backward compatibility a bit.

Well, people shouldn't rely on bugs.  Otherwise we would never be able to fix 
bugs, lest someone relies on it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

Regarding fixing (1), I'm worrying about backward compatibility a bit. Some 
people who discovered that behavior might rely on such "move" semantics and 
expect that the redirected descriptor is not leaked into the child. OTOH, since 
descriptors are non-inheritable by default since 3.4, it might be less of an 
issue now. What do you think?

Otherwise, yes, I'd like to fix this, but fixing is a bit trickier than it may 
seem because sometimes descriptors are duplicated in _posixsubprocess.c (in 
case of low fds), so we need to track the ownership properly. I've already 
performed some work and discovered another issue: in a corner case involving 
closed standard fds, pipes created by subprocess may leak into the child and/or 
an incorrect redirection may occur. Since I can't completely fix this issue 
without affecting a discovered one, I think I'll do the opposite: file a new 
issue, fix it, and then go back to this one.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Nitish]

Change by Nitish :


--
nosy: +nitishch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Thanks for the report and the investigation!

> 1. The fact that file descriptors specified for redirection are closed (even 
> with close_fds=False) unless in range [0, 2] is not documented and not tested.

That sounds like a bug to me, so we should fix it if decently possible.

> 2. pass_fds interaction with (1) should be fixed and documented. We may 
> either raise ValueError if pass_fds contains a redirected fd or don't close 
> such a redirected fd.

Same here, it seems like a bug which deserves fixing.

Do you want to submit a PR for this?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32270] subprocess closes redirected fds even if they are in pass_fds

[Alexey Izbyshev]

New submission from Alexey Izbyshev :

Demonstration:

$ cat test.py
import os
import subprocess
import sys

fd = os.dup(sys.stdout.fileno())
subprocess.call([sys.executable, '-c',
 'import sys;'
 'print("Hello stdout");'
 'print("Hello fd", file=open(int(sys.argv[1]), "w"))',
 str(fd)],
stdout=fd,
pass_fds=[fd])


$ python3 test.py
Hello stdout
Traceback (most recent call last):
  File "", line 1, in 
OSError: [Errno 9] Bad file descriptor

I see two issues here:

1. The fact that file descriptors specified for redirection are closed (even 
with close_fds=False) unless in range [0, 2] is not documented and not tested. 
If I change the corresponding code in _posixsubprocess.c to close them only if 
they are ends of pipes created by subprocess itself, all tests still pass. 
Also, shells behave differently: e.g., in command 'echo 3>&1' fd 3 is 
duplicated but not closed in the child, so subprocess behavior may be not what 
people expect.

2. pass_fds interaction with (1) should be fixed and documented. We may either 
raise ValueError if pass_fds contains a redirected fd or don't close such a 
redirected fd.

Please provide your thoughts.

--
components: Library (Lib)
files: test.py
messages: 307962
nosy: gregory.p.smith, izbyshev, pitrou, vstinner
priority: normal
severity: normal
status: open
title: subprocess closes redirected fds even if they are in pass_fds
type: behavior
versions: Python 3.6, Python 3.7
Added file: https://bugs.python.org/file47329/test.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com