merge from 3.5 - Fixes Issue #26373: subprocess.Popen.communicate
now correctly ignores BrokenPipeError when the child process dies
before .communicate() is called in more (all?) circumstances.
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 716f7ff..98f339e 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1036,8 +1036,7 @@
try:
self.stdin.write(input)
except BrokenPipeError:
- # communicate() must ignore broken pipe error
- pass
+ pass # communicate() must ignore broken pipe errors.
except OSError as e:
if e.errno == errno.EINVAL and self.poll() is not None:
# Issue #19612: On Windows, stdin.write() fails with EINVAL
@@ -1045,7 +1044,15 @@
pass
else:
raise
- self.stdin.close()
+ try:
+ self.stdin.close()
+ except BrokenPipeError:
+ pass # communicate() must ignore broken pipe errors.
+ except OSError as e:
+ if e.errno == errno.EINVAL and self.poll() is not None:
+ pass
+ else:
+ raise
def communicate(self, input=None, timeout=None):
"""Interact with process: Send data to stdin. Read data from
@@ -1691,9 +1698,15 @@
if self.stdin and not self._communication_started:
# Flush stdio buffer. This might block, if the user has
# been writing to .stdin in an uncontrolled fashion.
- self.stdin.flush()
+ try:
+ self.stdin.flush()
+ except BrokenPipeError:
+ pass # communicate() must ignore BrokenPipeError.
if not input:
- self.stdin.close()
+ try:
+ self.stdin.close()
+ except BrokenPipeError:
+ pass # communicate() must ignore BrokenPipeError.
stdout = None
stderr = None
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index cb0e292..78eee9e 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1,4 +1,5 @@
import unittest
+from unittest import mock
from test import support
import subprocess
import sys
@@ -1237,6 +1238,52 @@
fds_after_exception = os.listdir(fd_directory)
self.assertEqual(fds_before_popen, fds_after_exception)
+ def test_communicate_BrokenPipeError_stdin_close(self):
+ # By not setting stdout or stderr or a timeout we force the fast path
+ # that just calls _stdin_write() internally due to our mock.
+ proc = subprocess.Popen([sys.executable, '-c', 'pass'])
+ with proc, mock.patch.object(proc, 'stdin') as mock_proc_stdin:
+ mock_proc_stdin.close.side_effect = BrokenPipeError
+ proc.communicate() # Should swallow BrokenPipeError from close.
+ mock_proc_stdin.close.assert_called_with()
+
+ def test_communicate_BrokenPipeError_stdin_write(self):
+ # By not setting stdout or stderr or a timeout we force the fast path
+ # that just calls _stdin_write() internally due to our mock.
+ proc = subprocess.Popen([sys.executable, '-c', 'pass'])
+ with proc, mock.patch.object(proc, 'stdin') as mock_proc_stdin:
+ mock_proc_stdin.write.side_effect = BrokenPipeError
+ proc.communicate(b'stuff') # Should swallow the BrokenPipeError.
+ mock_proc_stdin.write.assert_called_once_with(b'stuff')
+ mock_proc_stdin.close.assert_called_once_with()
+
+ def test_communicate_BrokenPipeError_stdin_flush(self):
+ # Setting stdin and stdout forces the ._communicate() code path.
+ # python -h exits faster than python -c pass (but spams stdout).
+ proc = subprocess.Popen([sys.executable, '-h'],
+ stdin=subprocess.PIPE,
+ stdout=subprocess.PIPE)
+ with proc, mock.patch.object(proc, 'stdin') as mock_proc_stdin, \
+ open('/dev/null', 'wb') as dev_null:
+ mock_proc_stdin.flush.side_effect = BrokenPipeError
+ # because _communicate registers a selector using proc.stdin...
+ mock_proc_stdin.fileno.return_value = dev_null.fileno()
+ # _communicate() should swallow BrokenPipeError from flush.
+ proc.communicate(b'stuff')
+ mock_proc_stdin.flush.assert_called_once_with()
+
+ def test_communicate_BrokenPipeError_stdin_close_with_timeout(self):
+ # Setting stdin and stdout forces the ._communicate() code path.
+ # python -h exits faster than python -c pass (but spams stdout).
+ proc = subprocess.Popen([sys.executable, '-h'],
+ stdin=subprocess.PIPE,
+ stdout=subprocess.PIPE)
+ with proc, mock.patch.object(proc, 'stdin') as mock_proc_stdin:
+ mock_proc_stdin.close.side_effect = BrokenPipeError
+ # _communicate() should swallow BrokenPipeError from close.
+ proc.communicate(timeout=999)
+ mock_proc_stdin.close.assert_called_once_with()
+
class RunFuncTestCase(BaseTestCase):
def run_python(self, code, **kwargs):
diff --git a/Misc/NEWS b/Misc/NEWS
index a05a4bc..2288bec 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -27,6 +27,10 @@
Library
-------
+- Issue #26373: subprocess.Popen.communicate now correctly ignores
+ BrokenPipeError when the child process dies before .communicate()
+ is called in more/all circumstances.
+
- signal, socket, and ssl module IntEnum constant name lookups now return a
consistent name for values having multiple names. Ex: signal.Signals(6)
now refers to itself as signal.SIGALRM rather than flipping between that