[issue7213] Popen.subprocess change close_fds default to True

Changes by Jakub Wilk : -- nosy: +jwilk ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/m

[issue7213] Popen.subprocess change close_fds default to True

STINNER Victor added the comment: test_pipe_cloexec_unix_tools() is specific to UNIX/BSD because it requires cat and grep programs. You should try to reuse the Python interpreter to have a portable test (eg. working on Windows), as you did with fd_status.py. +data = b'aaa

[issue7213] Popen.subprocess change close_fds default to True

STINNER Victor added the comment: subprocess-cloexec-atomic-py3k.patch: +case $ac_sys_system in + GNU*|Linux*) + AC_CHECK_FUNC(pipe2, AC_DEFINE(HAVE_PIPE2, 1, [Define if the OS supports pipe2()]), ) +esac I think that you can remove the test on the OS name. AC_CHECK_FUNC() doesn't hur

[issue7213] Popen.subprocess change close_fds default to True

STINNER Victor added the comment: > subprocess-cloexec-atomic-py3k-tests2-close_fds.patch adds a test > called [test_close_fds] to Win32ProcessTestCase ... Oops, forget my last comment, I didn't applied the patches in the right order. There are too much patches :-p Can you try to create one un

[issue7213] Popen.subprocess change close_fds default to True

STINNER Victor added the comment: subprocess-cloexec-atomic-py3k-tests2-close_fds.patch adds a test called to Win32ProcessTestCase which is specific to Windows. And this class has already a test with the same name. You should move your test to ProcessTestCase (and so it will test the C implem

[issue7213] Popen.subprocess change close_fds default to True

Milko Krachounov added the comment: I add a patch that tests close_fds (there's no test for close_fds), that requires the tests1 patch. By the way, should there be a test for the atomicity of the operations? -- Added file: http://bugs.python.org/file20013/subprocess-cloexec-atomic-py

[issue7213] Popen.subprocess change close_fds default to True

Changes by Milko Krachounov : Added file: http://bugs.python.org/file20009/subprocess-cloexec-atomic-py3k-tests1.patch ___ Python tracker ___

[issue7213] Popen.subprocess change close_fds default to True

Changes by Milko Krachounov : Removed file: http://bugs.python.org/file20008/subprocess-cloexec-atomic-py3k-tests1.patch ___ Python tracker ___ __

[issue7213] Popen.subprocess change close_fds default to True

Changes by Milko Krachounov : Added file: http://bugs.python.org/file20008/subprocess-cloexec-atomic-py3k-tests1.patch ___ Python tracker ___

[issue7213] Popen.subprocess change close_fds default to True

Changes by Milko Krachounov : Removed file: http://bugs.python.org/file20007/subprocess-cloexec-atomic-py3k-tests1.patch ___ Python tracker ___ __

[issue7213] Popen.subprocess change close_fds default to True

Milko Krachounov added the comment: I attached unit tests that test that cloexec is properly set. I can't test my tests too well with the unpatched version because runtests.sh is too complicated to use, and doesn't print any useful output by default. -- Added file: http://bugs.python

[issue7213] Popen.subprocess change close_fds default to True

STINNER Victor added the comment: Can you add a test to your patch? -- nosy: +haypo ___ Python tracker ___ ___ Python-bugs-list mailin

[issue7213] Popen.subprocess change close_fds default to True

Milko Krachounov added the comment: I created another patch that attempts to create the pipes atomically. On GNU/Linux, if pipe2 is available, it uses it to create the pipes, and there is no race. On other POSIX platforms, pipe and fcntl are called without releasing the GIL - relying on the f

[issue7213] Popen.subprocess change close_fds default to True

Ned Deily added the comment: (Adding the 3.2 release manager: a potential release blocker?) -- nosy: +georg.brandl, ned.deily ___ Python tracker ___ _

[issue7213] Popen.subprocess change close_fds default to True

Milko Krachounov added the comment: It's almost exactly the same race condition as the one described in issue 2320. The pipes are created and stay without the CLOEXEC flag for a while (until the process has been forked and fcntl has been called). During that time another thread can launch a s

[issue7213] Popen.subprocess change close_fds default to True

Giovanni Bajo added the comment: Would you mind elaborating on where is the race condition? -- ___ Python tracker ___ ___ Python-bugs-

[issue7213] Popen.subprocess change close_fds default to True

Milko Krachounov added the comment: > I'm +1 on it, but I think it should be the default; instead, > your proposed patch adds a new argument to the public API. Why do you > think it's necessary to do so? I don't think it's necessary. I put it there because when I was testing I thought it mig

[issue7213] Popen.subprocess change close_fds default to True

Changes by Milko Krachounov : Removed file: http://bugs.python.org/file1/subprocess-cloexec-py3k.patch ___ Python tracker ___ ___ Python-bu

[issue7213] Popen.subprocess change close_fds default to True

Giovanni Bajo added the comment: Setting CLOEXEC on the pipes seems like a very good fix for this bug. I'm +1 on it, but I think it should be the default; instead, your proposed patch adds a new argument to the public API. Why do you think it's necessary to do so? At the same time, we need a

[issue7213] Popen.subprocess change close_fds default to True

Milko Krachounov added the comment: The cloexec approach still doesn't help with issue 2320. In fact, with threading and people calling subprocess from multiple threads, *this* issue wouldn't be fixed with my patch either unless mutexes are used. It's impossible to avoid a race here with thre

[issue7213] Popen.subprocess change close_fds default to True

Milko Krachounov added the comment: I'd offer two ideas. 1. Add a constant DISREGARD_FDS to the subprocess module could help. It would allow the user to specify his intent, and let the implementation choose the best action. Popen(..., close_fds=subprocess.DISREGARD_FDS) would mean that any f

[issue7213] Popen.subprocess change close_fds default to True

Giovanni Bajo added the comment: Hi Gregory, I saw your commit here: http://code.activestate.com/lists/python-checkins/91914/ This basically means that in 3.2 it is mandatory to specify close_fds to avoid a DeprecationWarning. *BUT* there is no good value that works both on Windows and Linux

[issue7213] Popen.subprocess change close_fds default to True

Paul Moore added the comment: This bug appears to be Unix-only. On Windows: >>> from subprocess import * >>> p1 = Popen(['cat'], stdin=PIPE, stdout=PIPE) >>> p2 = Popen(['grep', 'a'], stdin=p1.stdout, stdout=PIPE) >>> p1.stdin.write("\n") >>> p1.stdin.close() >>> p2.stdout.read(

[issue7213] Popen.subprocess change close_fds default to True

New submission from Milko Krachounov : Currently, close_fds defaults to False. The are few cases in which one would want to leave the fds open, in all the rest leaving them open can lead to unpleasant side effects. For example, the following doesn't work: >>> p1 = Popen(['cat'], stdin=PIPE, stdo