From: Gregory P. Smith <greg@krypto.org>
Date: Sat, 23 Mar 2013 18:54:22 +0000 (-0700)
Subject: Fixes issue #17488: Change the subprocess.Popen bufsize parameter default value
X-Git-Tag: v3.4.0a1~1102^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a1b9ed32ee69c9204959d5db2475001d199b3e50;p=python

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.
---

a1b9ed32ee69c9204959d5db2475001d199b3e50
diff --cc Doc/library/subprocess.rst
index 92b21b5ae9,59ee13f63d..a72ca44022
--- a/Doc/library/subprocess.rst
+++ b/Doc/library/subprocess.rst
@@@ -428,17 -356,20 +428,20 @@@ functions
        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
++   .. 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 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.
++      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 --cc Lib/subprocess.py
index 773f3e8cd0,725564567c..689046ebbf
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@@ -722,10 -649,8 +722,10 @@@ class Popen(object)
          _cleanup()
  
          self._child_created = False
 +        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 --cc Lib/test/test_subprocess.py
index e8e74edca6,1a50de3a6f..dd720fecf2
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@@ -82,6 -79,28 +82,34 @@@ class PopenExecuteChildRaises(subproces
  
  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",