diff options
| -rw-r--r-- | Lib/subprocess.py | 23 | ||||
| -rw-r--r-- | Lib/test/test_subprocess.py | 47 | ||||
| -rw-r--r-- | Misc/NEWS | 4 |
3 files changed, 69 insertions, 5 deletions
diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 716f7fff53..98f339ee5c 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1036,8 +1036,7 @@ class Popen(object): 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 @@ class Popen(object): 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 @@ class Popen(object): 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 cb0e2927de..78eee9e988 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 @@ class ProcessTestCase(BaseTestCase): 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): @@ -27,6 +27,10 @@ Core and Builtins 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 |