]> granicus.if.org Git - python/commitdiff
Fixes issue #17488: Change the subprocess.Popen bufsize parameter default value
authorGregory P. Smith <greg@krypto.org>
Sat, 23 Mar 2013 18:54:22 +0000 (11:54 -0700)
committerGregory P. Smith <greg@krypto.org>
Sat, 23 Mar 2013 18:54:22 +0000 (11:54 -0700)
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.

1  2 
Doc/library/subprocess.rst
Lib/subprocess.py
Lib/test/test_subprocess.py
Misc/NEWS

index 92b21b5ae90e35241406fdd689ed5f91a7919e63,59ee13f63ddd053afc5617e66423e445f733756c..a72ca44022885a3d5eed4317a9376611475b6715
@@@ -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
index 773f3e8cd07446a7c81976b8d1d3f184481e5ddf,725564567c1427fe400bf978bb375497ade044bc..689046ebbff35a1c649b8a5ef72dea25799174ef
@@@ -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")
  
index e8e74edca6ac4d204e456b16d6a0347e335bcb51,1a50de3a6fe1618d96115ace81ceb8f7bcd2f30e..dd720fecf2f6fe242ce8daa909409d6403f10bf3
@@@ -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",
diff --cc Misc/NEWS
Simple merge