diff options
| -rw-r--r-- | Doc/library/subprocess.rst | 18 | ||||
| -rw-r--r-- | Lib/subprocess.py | 3 | ||||
| -rw-r--r-- | Lib/test/test_subprocess.py | 33 | ||||
| -rw-r--r-- | Misc/NEWS | 3 |
4 files changed, 50 insertions, 7 deletions
diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 854993c..b2238f0 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -406,12 +406,18 @@ functions. Read the `Security Considerations`_ section before using ``shell=True``. - *bufsize* will be supplied as the corresponding argument to the :func:`open` - function when creating the stdin/stdout/stderr pipe file objects: :const:`0` - means unbuffered (read and write are one system call and can return short), - :const:`1` means line buffered, any other positive value means use a buffer - of approximately that size. A negative bufsize (the default) means the - system default of io.DEFAULT_BUFFER_SIZE will be used. + *bufsize* will be supplied as the corresponding argument to the + :func:`open` function when creating the stdin/stdout/stderr pipe + file objects: + + - :const:`0` means unbuffered (read and write are one + system call and can return short) + - :const:`1` means line buffered + (only usable if ``universal_newlines=True`` i.e., in a text mode) + - any other positive value means use a buffer of approximately that + size + - negative bufsize (the default) means the system default of + io.DEFAULT_BUFFER_SIZE will be used. .. versionchanged:: 3.3.1 *bufsize* now defaults to -1 to enable buffering by default to match the diff --git a/Lib/subprocess.py b/Lib/subprocess.py index ddc033a..bc0ef0b 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -837,7 +837,8 @@ class Popen(object): if p2cwrite != -1: self.stdin = io.open(p2cwrite, 'wb', bufsize) if universal_newlines: - self.stdin = io.TextIOWrapper(self.stdin, write_through=True) + self.stdin = io.TextIOWrapper(self.stdin, write_through=True, + line_buffering=(bufsize == 1)) if c2pread != -1: self.stdout = io.open(c2pread, 'rb', bufsize) if universal_newlines: diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index d0ab718..9616da0 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1008,6 +1008,39 @@ class ProcessTestCase(BaseTestCase): p = subprocess.Popen([sys.executable, "-c", "pass"], bufsize=None) self.assertEqual(p.wait(), 0) + def _test_bufsize_equal_one(self, line, expected, universal_newlines): + # subprocess may deadlock with bufsize=1, see issue #21332 + with subprocess.Popen([sys.executable, "-c", "import sys;" + "sys.stdout.write(sys.stdin.readline());" + "sys.stdout.flush()"], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + bufsize=1, + universal_newlines=universal_newlines) as p: + p.stdin.write(line) # expect that it flushes the line in text mode + os.close(p.stdin.fileno()) # close it without flushing the buffer + read_line = p.stdout.readline() + try: + p.stdin.close() + except OSError: + pass + p.stdin = None + self.assertEqual(p.returncode, 0) + self.assertEqual(read_line, expected) + + def test_bufsize_equal_one_text_mode(self): + # line is flushed in text mode with bufsize=1. + # we should get the full line in return + line = "line\n" + self._test_bufsize_equal_one(line, line, universal_newlines=True) + + def test_bufsize_equal_one_binary_mode(self): + # line is not flushed in binary mode with bufsize=1. + # we should get empty response + line = b'line' + os.linesep.encode() # assume ascii-based locale + self._test_bufsize_equal_one(line, b'', universal_newlines=False) + def test_leaking_fds_on_error(self): # see bug #5179: Popen leaks file descriptors to PIPEs if # the child fails to execute; this will eventually exhaust @@ -35,6 +35,9 @@ Library - Issue #22423: Unhandled exception in thread no longer causes unhandled AttributeError when sys.stderr is None. +- Issue #21332: Ensure that ``bufsize=1`` in subprocess.Popen() selects + line buffering, rather than block buffering. Patch by Akira Li. + - Issue #21091: Fix API bug: email.message.EmailMessage.is_attachment is now a method. Since EmailMessage is provisional, we can change the API in a maintenance release, but we use a trick to remain backward compatible with |