[Python-checkins] cpython (merge 3.3 -> default): Fixes issue #17488: Change the subprocess.Popen bufsize parameter default value

gregory.p.smith python-checkins at python.org
Sat Mar 23 20:00:49 CET 2013


http://hg.python.org/cpython/rev/3031d69f94ef
changeset:   82918:3031d69f94ef
parent:      82915:60ec59b5d717
parent:      82917:4c2fc172afcc
user:        Gregory P. Smith <greg at krypto.org>
date:        Sat Mar 23 12:00:00 2013 -0700
summary:
  Fixes issue #17488: Change the subprocess.Popen bufsize parameter default value
from unbuffered (0) to buffering (-1) to match the behavior existing code
expects and match the behavior of the subprocess module in Python 2 to avoid
introducing hard to track down bugs.

files:
  Doc/library/subprocess.rst  |  23 +++++++++++--------
  Lib/subprocess.py           |  18 ++++++++--------
  Lib/test/test_subprocess.py |  28 +++++++++++++++++++++++++
  Misc/NEWS                   |   5 ++++
  4 files changed, 55 insertions(+), 19 deletions(-)


diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst
--- a/Doc/library/subprocess.rst
+++ b/Doc/library/subprocess.rst
@@ -356,7 +356,7 @@
 functions.
 
 
-.. class:: Popen(args, bufsize=0, executable=None, stdin=None, stdout=None, \
+.. class:: Popen(args, bufsize=-1, executable=None, stdin=None, stdout=None, \
                  stderr=None, preexec_fn=None, close_fds=True, shell=False, \
                  cwd=None, env=None, universal_newlines=False, \
                  startupinfo=None, creationflags=0, restore_signals=True, \
@@ -428,17 +428,20 @@
       untrusted input.  See the warning under :ref:`frequently-used-arguments`
       for details.
 
-   *bufsize*, if given, has the same meaning as the corresponding argument to the
-   built-in open() function: :const:`0` means unbuffered, :const:`1` means line
-   buffered, any other positive value means use a buffer of (approximately) that
-   size.  A negative *bufsize* means to use the system default, which usually means
-   fully buffered.  The default value for *bufsize* is :const:`0` (unbuffered).
+   *bufsize* will be supplied as the corresponding argument to the :meth:`io.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.
 
-   .. note::
+   .. versionchanged:: 3.2.4, 3.3.1
 
-      If you experience performance issues, it is recommended that you try to
-      enable buffering by setting *bufsize* to either -1 or a large enough
-      positive value (such as 4096).
+      *bufsize* now defaults to -1 to enable buffering by default to match the
+      behavior that most code expects.  In 3.2.0 through 3.2.3 and 3.3.0 it
+      incorrectly defaulted to :const:`0` which was unbuffered and allowed
+      short reads.  This was unintentional and did not match the behavior of
+      Python 2 as most code expected.
 
    The *executable* argument specifies a replacement program to execute.   It
    is very seldom needed.  When ``shell=False``, *executable* replaces the
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -25,7 +25,7 @@
 ===========================
 This module defines one class called Popen:
 
-class Popen(args, bufsize=0, executable=None,
+class Popen(args, bufsize=-1, executable=None,
             stdin=None, stdout=None, stderr=None,
             preexec_fn=None, close_fds=True, shell=False,
             cwd=None, env=None, universal_newlines=False,
@@ -56,12 +56,12 @@
 way: The list2cmdline is designed for applications using the same
 rules as the MS C runtime.
 
-bufsize, if given, has the same meaning as the corresponding argument
-to the built-in open() function: 0 means unbuffered, 1 means line
-buffered, any other positive value means use a buffer of
-(approximately) that size.  A negative bufsize means to use the system
-default, which usually means fully buffered.  The default value for
-bufsize is 0 (unbuffered).
+bufsize will be supplied as the corresponding argument to the io.open()
+function when creating the stdin/stdout/stderr pipe file objects:
+0 means unbuffered (read & write are one system call and can return short),
+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.
 
 stdin, stdout and stderr specify the executed programs' standard
 input, standard output and standard error file handles, respectively.
@@ -709,7 +709,7 @@
 
 
 class Popen(object):
-    def __init__(self, args, bufsize=0, executable=None,
+    def __init__(self, args, bufsize=-1, executable=None,
                  stdin=None, stdout=None, stderr=None,
                  preexec_fn=None, close_fds=_PLATFORM_DEFAULT_CLOSE_FDS,
                  shell=False, cwd=None, env=None, universal_newlines=False,
@@ -723,7 +723,7 @@
         self._input = None
         self._communication_started = False
         if bufsize is None:
-            bufsize = 0  # Restore default
+            bufsize = -1  # Restore default
         if not isinstance(bufsize, int):
             raise TypeError("bufsize must be an integer")
 
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -82,6 +82,34 @@
 
 class ProcessTestCase(BaseTestCase):
 
+    def test_io_buffered_by_default(self):
+        p = subprocess.Popen([sys.executable, "-c", "import sys; sys.exit(0)"],
+                             stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE)
+        try:
+            self.assertIsInstance(p.stdin, io.BufferedIOBase)
+            self.assertIsInstance(p.stdout, io.BufferedIOBase)
+            self.assertIsInstance(p.stderr, io.BufferedIOBase)
+        finally:
+            p.stdin.close()
+            p.stdout.close()
+            p.stderr.close()
+            p.wait()
+
+    def test_io_unbuffered_works(self):
+        p = subprocess.Popen([sys.executable, "-c", "import sys; sys.exit(0)"],
+                             stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE, bufsize=0)
+        try:
+            self.assertIsInstance(p.stdin, io.RawIOBase)
+            self.assertIsInstance(p.stdout, io.RawIOBase)
+            self.assertIsInstance(p.stderr, io.RawIOBase)
+        finally:
+            p.stdin.close()
+            p.stdout.close()
+            p.stderr.close()
+            p.wait()
+
     def test_call_seq(self):
         # call() function with sequence argument
         rc = subprocess.call([sys.executable, "-c",
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -294,6 +294,11 @@
 Library
 -------
 
+- Issue #17488: Change the subprocess.Popen bufsize parameter default value
+  from unbuffered (0) to buffering (-1) to match the behavior existing code
+  expects and match the behavior of the subprocess module in Python 2 to avoid
+  introducing hard to track down bugs.
+
 - Issue #17521: Corrected non-enabling of logger following two calls to
   fileConfig().
 

-- 
Repository URL: http://hg.python.org/cpython


More information about the Python-checkins mailing list