Cleaning up a subprocess with a broken pipe

[vadmium]

BPO	21619
Nosy	@pitrou, @vstinner, @davidmalcolm, @4kir4, @vadmium, @serhiy-storchaka
Files	pipe-cleanup.patch overflow-pipe-test.patch overflow-pipe-test-2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-03-08.08:42:12.791>
created_at = <Date 2014-05-31.12:57:05.282>
labels = ['library', 'performance']
title = 'Cleaning up a subprocess with a broken pipe'
updated_at = <Date 2015-03-08.08:42:12.790>
user = 'https://github.com/vadmium'

bugs.python.org fields:

activity = <Date 2015-03-08.08:42:12.790>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-03-08.08:42:12.791>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2014-05-31.12:57:05.282>
creator = 'martin.panter'
dependencies = []
files = ['37463', '38364', '38386']
hgrepos = []
issue_num = 21619
keywords = ['patch']
message_count = 26.0
messages = ['219450', '232732', '236873', '236874', '236875', '236877', '236949', '236950', '236952', '236961', '236964', '237005', '237098', '237105', '237109', '237116', '237119', '237121', '237124', '237226', '237249', '237409', '237484', '237514', '237515', '237516']
nosy_count = 9.0
nosy_names = ['pitrou', 'vstinner', 'Arfrever', 'dmalcolm', 'akira', 'rosslagerwall', 'python-dev', 'martin.panter', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue21619'
versions = ['Python 3.4', 'Python 3.5']

[vadmium]

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(b"buffered 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/5    00:00:00 python3
15869 pts/5    00:00:00 true <defunct>
15873 pts/5    00:00:00 ps
32227 pts/5    00: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 bpo-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()

[vadmium]

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.

[python-dev]

New changeset b5e9ddbdd4a7 by Serhiy Storchaka in branch '3.4':
Issue bpo-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 bpo-21619: Popen objects no longer leave a zombie after exit in the with
https://hg.python.org/cpython/rev/cdac249808a8

[serhiy-storchaka]

Thank you for your contribution Martin.

[vstinner]

Why not ignoring BrokenPipeError like communicate()?

[python-dev]

New changeset 1b4d916329e7 by Serhiy Storchaka in branch '3.4':
Fixed a test for issue bpo-21619 on Windows.
https://hg.python.org/cpython/rev/1b4d916329e7

New changeset eae459e35cb9 by Serhiy Storchaka in branch 'default':
Fixed a test for issue bpo-21619 on Windows.
https://hg.python.org/cpython/rev/eae459e35cb9

[serhiy-storchaka]

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() (bpo-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__

---

[serhiy-storchaka]

Why not ignoring BrokenPipeError like communicate()?

Why communicate() ignores BrokenPipeError?

[serhiy-storchaka]

It was added in bpo-10963. I don't know if this way is applicable to this issue.

[vstinner]

Le dimanche 1 mars 2015, Serhiy Storchaka <report@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__?