Gregory P. Smith added the comment:
Thanks! Backported to subprocess32 in
https://code.google.com/p/python-subprocess32/source/detail?r=4ba30d9c64296ea0d2959790ab22d0f1a2678064
--
___
Python tracker rep...@bugs.python.org
Roundup Robot added the comment:
New changeset f2b023135b1b by Charles-François Natali in branch '2.7':
Issue #18763: subprocess: The file descriptors are now closed after calling the
http://hg.python.org/cpython/rev/f2b023135b1b
New changeset 4c52d4bac03c by Charles-François Natali in branch
Changes by Charles-François Natali cf.nat...@gmail.com:
--
resolution: - fixed
stage: commit review - committed/rejected
status: open - closed
versions: +Python 2.7, Python 3.3
___
Python tracker rep...@bugs.python.org
Charles-François Natali added the comment:
Here's a patch with a more robust test: the previous test worked, but assume
that only stdin, stdout and stderr FDs would be open in the child process. This
might not hold anymore in a near future (/dev/urandom persistent FD).
The new test is much
STINNER Victor added the comment:
I don't understand why os.dup2() is more reliable than os.pipe() for a unit
test?
Is subprocess_close-default-1.diff portable? test_os uses hasattr(os, dup2).
In Modules/posixmodule.c, it looks like the function is always defined!?
--
Charles-François Natali added the comment:
I don't understand why os.dup2() is more reliable than os.pipe() for a unit
test?
I use dup2() because it allows me to specify a target FD, so the
parent can know precisely which FD was opened by the preexec hook, and
check it's closed in the child
STINNER Victor added the comment:
the parent can know precisely which FD was opened by
the preexec hook, and check it's closed in the child process.
Oh ok, I see:
+self.assertNotIn(fd, remaining_fds)
You might also add a check:
self.assertLessEqual(remaining_fds, {0, 1, 2})
Charles-François Natali added the comment:
You might also add a check:
self.assertLessEqual(remaining_fds, {0, 1, 2})
Well no, because the interpreter might have other FDs open than stdin,
stdout and stderr.
--
___
Python tracker
STINNER Victor added the comment:
Close_fds is not supposed to close them?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18763
___
___
Charles-François Natali added the comment:
Close_fds is not supposed to close them?
Yes, but some new fds might be open in the child process, e.g. for /dev/urandom.
That's why it's better to check that this precise FD is closed. Of
course, if the /dev/urandom FD gets assigned the same FD as
STINNER Victor added the comment:
Yes, but some new fds might be open in the child process, e.g. for
/dev/urandom.
The issue #18756 is not implemented not, and I'm still opposed to change
os.random() to use persistent FD :)
Ok, no problem for not checking others FD, it's not the purpose of
Charles-François Natali added the comment:
Using self.assertLessEqual() would provide better message on error.
Done.
You don't want to fix Python 2.7 and 3.3? It is a bug in my opinion.
Patch for 2.7 attached (without test though, because 2.7 lacks a
fd_status.py helper, which would make
Changes by Charles-François Natali cf.nat...@gmail.com:
--
nosy: +sbt
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18763
___
___
Python-bugs-list
STINNER Victor added the comment:
Your change looks perfectly valid, except this minor nit:
+self.assertTrue(remaining_fds = set([0, 1, 2]))
Using self.assertLessEqual() would provide better message on error.
Note: with the PEP 446, you would not have to care of closing file
STINNER Victor added the comment:
You don't want to fix Python 2.7 and 3.3? It is a bug in my opinion.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18763
___
Charles-François Natali added the comment:
With patch :)
--
Added file: http://bugs.python.org/file31342/subprocess_close.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18763
___diff -r
New submission from Charles-François Natali:
Currently, when passed close_fds=True, the subprocess module closes FDs before
calling preexec_fn (if provided). This can be an issue if preexec_fn opens some
file descriptors, which would then be inherited in the child process.
Here's a patch with
Gregory P. Smith added the comment:
I don't see a patch attached, but I do not recall any good reason off the top
of my head for preexec_fn to be called as late as it is. Moving it up to be
called before the fd closing loop makes sense as a bug fix.
All bets are off when it comes to safe
18 matches
Mail list logo
