[issue21619] Cleaning up a subprocess with a broken pipe
Changes by Serhiy Storchaka storch...@gmail.com: -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Martin Panter added the comment: Sure, new version is fine by me -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: I think we should add __enter__ for consistency. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Changes by Serhiy Storchaka storch...@gmail.com: Added file: http://bugs.python.org/file38386/overflow-pipe-test-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Roundup Robot added the comment: New changeset 4ea40dc3d26d by Serhiy Storchaka in branch '3.4': Issue #21619: Cleaned up test_broken_pipe_cleanup. https://hg.python.org/cpython/rev/4ea40dc3d26d New changeset 41ce95a5b2d8 by Serhiy Storchaka in branch 'default': Issue #21619: Cleaned up test_broken_pipe_cleanup. https://hg.python.org/cpython/rev/41ce95a5b2d8 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
STINNER Victor added the comment: overflow-pipe-test.patch looks good to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Martin Panter added the comment: Thanks for getting the test working. Just to tidy things up here I would like to get rid of my stdout signalling in the test, which is no longer needed and could be misleading. See overflow-pipe-test.patch. -- status: closed - open Added file: http://bugs.python.org/file38364/overflow-pipe-test.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: Looks as tests are fixed. Thank you Victor. -- resolution: - fixed stage: needs patch - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Roundup Robot added the comment: New changeset 77a978716517 by Victor Stinner in branch '3.4': Issue #21619: Try to fix test_broken_pipe_cleanup() https://hg.python.org/cpython/rev/77a978716517 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Changes by Serhiy Storchaka storch...@gmail.com: -- stage: - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Martin Panter added the comment: Aha! So perhaps Windows can accept a small amount of data into its pipe buffer even if we know the pipe has been broken. That kind of makes sense. Test case could be modified to: proc = subprocess.Popen([...], bufsize=support.PIPE_MAX_SIZE, stdin=subprocess.PIPE, stdout=subprocess.PIPE) proc.stdout.read() # Make sure subprocess has closed its input proc.stdin.write(bytes(support.PIPE_MAX_SIZE)) self.assertIsNone(proc.returncode) # Expect EPIPE under POSIX and EINVAL under Windows self.assertRaises(OSError, proc.__exit__, None, None, None) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
STINNER Victor added the comment: I opened the issue #23570: Change with subprocess.Popen(): (context manager) to ignore broken pipe error. FAIL: test_broken_pipe_cleanup (test.test_subprocess.ContextManagerTests) Serhiy: see existing test_communicate_epipe() and test_communicate_epipe_only_stdin() tests which are now reliable and portable. You should write more data (2**20 bytes) and set the buffer size to a value larger than the input data, to buffer all data, so the write occurs at stdin.close() in Popen.__exit__(). By the way, instead of an hardcoded value (2**20), support.PIPE_MAX_SIZE may be more appropriate. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: Could you provide a patch Martin? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
STINNER Victor added the comment: I would be safer to use a bufsize a little bit larger :-) proc = subprocess.Popen([...], bufsize=support.PIPE_MAX_SIZE * 2, stdin=subprocess.PIPE, stdout=subprocess.PIPE) ... proc.stdin.write(b'x' * support.PIPE_MAX_SIZE) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Akira Li added the comment: On Windows behavior http://stackoverflow.com/questions/23688492/oserror-errno-22-invalid-argument-in-subprocess -- nosy: +akira ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Martin Panter added the comment: Thanks for that link; the answer by Eryksun https://stackoverflow.com/questions/23688492/oserror-errno-22-invalid-argument-in-subprocess#28020096 is particularly enlightening. Apparently EINVAL actually represents an underlying broken pipe condition in Windows. Anyway, as far as the test is concerned, it doesn’t matter what particular exception is raised. This still doesn’t explain Serhiy’s observation that sometimes no exception is raised at all, https://bugs.python.org/issue21619#msg236949. Does Windows sometimes not report broken pipe errors, or is my signalling logic not working and there is a race condition? The whole point of the test is to trigger an exception when stdin is closed, so it would be nice to be able to reliably do that on Windows. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
STINNER Victor added the comment: A few months ago, I modified Popen.communicate() to handle EINVAL on Windows. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: The test still sporadically fails on Windows: http://buildbot.python.org/all/builders/x86%20Windows7%203.x/builds/9323/steps/test/logs/stdio == FAIL: test_broken_pipe_cleanup (test.test_subprocess.ContextManagerTests) Broken pipe error should not prevent wait() (Issue 21619) -- Traceback (most recent call last): File D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_subprocess.py, line 2515, in test_broken_pipe_cleanup self.assertRaises(OSError, proc.__exit__, None, None, None) AssertionError: OSError not raised by __exit__ -- -- status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: Why not ignoring BrokenPipeError like communicate()? Why communicate() ignores BrokenPipeError? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: It was added in issue10963. I don't know if this way is applicable to this issue. -- nosy: +dmalcolm, pitrou, rosslagerwall resolution: fixed - stage: resolved - ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
STINNER Victor added the comment: Le dimanche 1 mars 2015, Serhiy Storchaka rep...@bugs.python.org a écrit : Why communicate() ignores BrokenPipeError? It's more convinient. There is nothing useful you can do on pipe error in communicate(). For __exit__, what do you want to on broken pipe error? I did't check yet: the process always exit after __exit__? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: When you write to the file you don't want the error was silently ignored. with open(filename, 'w') as f: f.write(content) And also I don't want the error was silently ignored when write to the subprocess. with subprocess.Popen(cmd, universal_newlines=True, stdin=subprocess.PIPE) as p: p.stdin.write(content) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Martin Panter added the comment: It seems two different issues have popped up: ## 1. Windows behaviour ## Windows apparently doesn’t handle broken pipes consistently, sometimes raising an EINVAL error, and sometimes not raising any error. I don’t know if this is a problem with Python, or a quirk with Windows. (Just tried with Wine, and it always raises ENOSPC instead, but I guess that’s a Wine-specific quirk.) Perhaps we can change the offending test case to the following, at least until someone can investigate and explain what is going on: ... if mswindows: try: proc.__exit__(None, None, None) except OSError: # Sometimes raises EINVAL, sometimes no error pass else: self.assertRaises(BrokenPipeError, proc.__exit__, None, None, None) ... ## 2. Suppressing BrokenPipeError ## I don’t have any strong opinions. I think in the past when writing to a process’s input raises BrokenPipeError, I have tended to skip the writing and go on to analyse the process’s output and/or exit status (the same as if writing succeeded). Some arguments for raising BrokenPipeError (current behaviour): * No change in behaviour * Slightly simpler implementation * Consistent with the way stdin.close() works * Potentially more flexible if the user cares about handling a broken pipe specially. * Exiting the context manager does not return any data, so no return value is lost by raising an exception. In contrast, one of the reasons communicate() was changed to sometimes suppress the exception is so that the captured output is returned and not lost. Arguments for suppressing BrokenPipeError: * Consistent with the way communicate() is meant to work, according to Issue 10963. * Probably more useful in normal use cases * User can always explicitly call stdin.close() if they really want to handle the BrokenPipeError * Avoid possible confusion and hiding a more important exception, if one was already raised inside the context manager, for example if a subprocess crash was detected, or if stdin.write() already raised its own BrokenPipeError. Replying to Victor: “the process always exit after __exit__?”: __exit__ is meant to call wait(), so it only returns when the subprocess exits. It could potentially still be running despite closing its input pipe. This issue was originally about making sure __exit__ called wait() in all cases. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Roundup Robot added the comment: New changeset b5e9ddbdd4a7 by Serhiy Storchaka in branch '3.4': Issue #21619: Popen objects no longer leave a zombie after exit in the with https://hg.python.org/cpython/rev/b5e9ddbdd4a7 New changeset cdac249808a8 by Serhiy Storchaka in branch 'default': Issue #21619: Popen objects no longer leave a zombie after exit in the with https://hg.python.org/cpython/rev/cdac249808a8 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Serhiy Storchaka added the comment: Thank you for your contribution Martin. -- resolution: - fixed stage: patch review - resolved type: - resource usage versions: +Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
STINNER Victor added the comment: Why not ignoring BrokenPipeError like communicate()? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Changes by Serhiy Storchaka storch...@gmail.com: -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Changes by Serhiy Storchaka storch...@gmail.com: -- assignee: - serhiy.storchaka nosy: +serhiy.storchaka stage: - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Roundup Robot added the comment: New changeset 1b4d916329e7 by Serhiy Storchaka in branch '3.4': Fixed a test for issue #21619 on Windows. https://hg.python.org/cpython/rev/1b4d916329e7 New changeset eae459e35cb9 by Serhiy Storchaka in branch 'default': Fixed a test for issue #21619 on Windows. https://hg.python.org/cpython/rev/eae459e35cb9 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Changes by STINNER Victor victor.stin...@gmail.com: -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
Martin Panter added the comment: Here is a patch to fix this by calling wait() even if stdin.close() fails, including a test case. With my patch, the subprocess context manager __exit__() will still raise a BrokenPipeError, but no zombie will be left. -- keywords: +patch Added file: http://bugs.python.org/file37463/pipe-cleanup.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21619] Cleaning up a subprocess with a broken pipe
New submission from Martin Panter: The documentation for the “subprocess” module says that a “with” statement will “wait for” the process, implying that it does not leave a zombie. However this is not the case if there is buffered input data: $ python3 -Wall -bt -q import subprocess with subprocess.Popen((true,), stdin=subprocess.PIPE, bufsize=-1) as p: ... from time import sleep; sleep(1) # Wait for pipe to be broken ... p.stdin.write(bbuffered data) ... 13 Traceback (most recent call last): File stdin, line 3, in module File /usr/lib/python3.4/subprocess.py, line 899, in __exit__ self.stdin.close() BrokenPipeError: [Errno 32] Broken pipe # (Hit Ctrl-Z here) [1]+ Stopped python3 -Wall -bt -q [Exit 148] $ ps PID TTY TIME CMD 15867 pts/500:00:00 python3 15869 pts/500:00:00 true defunct 15873 pts/500:00:00 ps 32227 pts/500:00:10 bash Similarly, calling Popen.communicate() does not clean the process up either if there is buffered input data and the process has already exited. The documentation does not spell out how a broken pipe is handled in communicate(), but after reading Issue 10963 I see that in many other cases it is meant to be ignored. The best way to clean up a subprocess that I have come up with to close the pipe(s) and call wait() in two separate steps, such as: try: proc.stdin.close() except BrokenPipeError: pass proc.wait() -- components: Library (Lib) messages: 219450 nosy: vadmium priority: normal severity: normal status: open title: Cleaning up a subprocess with a broken pipe versions: Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21619 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
