[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

Gregory P. Smith added the comment: Thanks! Backported to subprocess32 in https://code.google.com/p/python-subprocess32/source/detail?r=4ba30d9c64296ea0d2959790ab22d0f1a2678064 -- ___ Python tracker _

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

Changes by Charles-François Natali : -- resolution: -> fixed stage: commit review -> committed/rejected status: open -> closed versions: +Python 2.7, Python 3.3 ___ Python tracker _

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 '3

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 o

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 th

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

STINNER Victor added the comment: Close_fds is not supposed to close them? -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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!? -- _

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 more

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 t

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 ___ ___ P

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 descripto

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

Changes by Charles-François Natali : -- nosy: +sbt ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

Charles-François Natali added the comment: With patch :) -- Added file: http://bugs.python.org/file31342/subprocess_close.diff ___ Python tracker ___diff -r 5d4fe1da2c90 Lib/test

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 beha

[issue18763] subprocess: file descriptors should be closed after preexec_fn is called

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 t