Milko Krachounov <pyt...@milko.3mhz.net> added the comment: OK, I have created new updated patches. I haven't combined them in one patch because some of the changes can be applied independently, the three patches can be cat'ed together if anyone sees separate patches a problem. ;)
I. Changes: * Now pipe2 would be used on all systems where available. * I don't depend on external grep and cat, but use utils shipped with the tests. II. The patches: 1. subprocess-00-subprocess_test_utils.patch This contains the fd_status.py and input_reader.py utilities that are needed by the tests. A separate file because they can be used for creating unit tests for issue 6559. In case these patches aren't accepted, it could be useful there. 2. subprocess-01-atomic_cloexec_pipe2.patch This file contains the changes that make the subprocess pipes created with the FD_CLOEXEC flag (atomically through pipe2) where possible. Can be used independently of the rest of the patches. 3. subprocess-02-cloexec_tests.patch This file contains the tests that verify that CLOEXEC is set on the pipes created by subprocess and this allows complex pipes between apps to work correctly. Requires subprocess-00-subprocess_test_utils.patch. It can be used for another implementation of CLOEXEC in case this implementation isn't accepted. With slight modification it can be used to test that pipes work fine with close_fds=True and no CLOEXEC. III. Atomicity and issue 2320: > I don't understand why do you talk about "atomicity". Do you test add > non-atomic > operations? Was subprocess atomic? > > If I understood correctly, you are fixing a specific issue which can be > called > something like "subprocess: close pipes on exec(), set FD_CLOEXEC flag to all > pipes", > and no more changing the default value of close_fds. Can you update the title > please? The CLOEXEC flag needs to be set atomically (or at least in a way that another subprocess won't start in the middle of it) so that issue 2320 is also resolved. This is why I suggested the use of pipe2. On systems where pipe2 isn't available, the patches rely on the coincidence that the GIL during both process execution and the pipe creation. I was wondering if a tests that verify for things like issue 2320 are wanted in general (there's no way to reliably test such, but there could be a test that does it with a certain probability). In short, yes, the suggested patches attempt to solve this issue without a change to the default, but also to resolve issue 2320 (changing the default would also resolve it). III. More issues: 1. What other possible situations exist causing the same issue that can be solved with close_fds=True and are caused by resources coming from other modules? Are any of those common? Maybe something involving os.openpty() and subprocess? Does anyone use those together? Are such issues worth worrying about (in other words, is changing the default still necessary if the subprocess pipes have the FD_CLOEXEC flag?) 2. What possible side-effects could the FD_CLOEXEC flag have? Any common use-case that is broken by the fact that the pipes are now with FD_CLOEXEC? 3. Can any changes be done to Python 2.7's subprocess? Can the issue be fixed there too, or it will have to remain as it is now? ---------- title: Popen.subprocess change close_fds default to True -> subprocess leaks open file descriptors between Popen instances causing hangs Added file: http://bugs.python.org/file20017/subprocess-00-subprocess_test_utils.patch _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue7213> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
