[Python-checkins] bpo-32236: open() emits RuntimeWarning if buffering=1 for binary mode (GH-4842)
Victor Stinner
webhook-mailer at python.org
Fri Oct 19 20:22:34 EDT 2018
https://github.com/python/cpython/commit/a2670565d8f5c502388378aba1fe73023fd8c8d4
commit: a2670565d8f5c502388378aba1fe73023fd8c8d4
branch: master
author: Alexey Izbyshev <izbyshev at ispras.ru>
committer: Victor Stinner <vstinner at redhat.com>
date: 2018-10-20T02:22:31+02:00
summary:
bpo-32236: open() emits RuntimeWarning if buffering=1 for binary mode (GH-4842)
If buffering=1 is specified for open() in binary mode, it is silently
treated as buffering=-1 (i.e., the default buffer size).
Coupled with the fact that line buffering is always supported in Python 2,
such behavior caused several issues (e.g., bpo-10344, bpo-21332).
Warn that line buffering is not supported if open() is called with
binary mode and buffering=1.
files:
A Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-50-40.bpo-32236.3RupnN.rst
M Doc/library/codecs.rst
M Lib/_pyio.py
M Lib/codecs.py
M Lib/subprocess.py
M Lib/test/support/__init__.py
M Lib/test/test_cmd_line_script.py
M Lib/test/test_file.py
M Lib/test/test_io.py
M Lib/test/test_subprocess.py
M Modules/_io/_iomodule.c
diff --git a/Doc/library/codecs.rst b/Doc/library/codecs.rst
index b9c04c241a8b..b323ab527d26 100644
--- a/Doc/library/codecs.rst
+++ b/Doc/library/codecs.rst
@@ -174,7 +174,7 @@ recommended approach for working with encoded text files, this module
provides additional utility functions and classes that allow the use of a
wider range of codecs when working with binary files:
-.. function:: open(filename, mode='r', encoding=None, errors='strict', buffering=1)
+.. function:: open(filename, mode='r', encoding=None, errors='strict', buffering=-1)
Open an encoded file using the given *mode* and return an instance of
:class:`StreamReaderWriter`, providing transparent encoding/decoding.
@@ -194,8 +194,8 @@ wider range of codecs when working with binary files:
*errors* may be given to define the error handling. It defaults to ``'strict'``
which causes a :exc:`ValueError` to be raised in case an encoding error occurs.
- *buffering* has the same meaning as for the built-in :func:`open` function. It
- defaults to line buffered.
+ *buffering* has the same meaning as for the built-in :func:`open` function.
+ It defaults to -1 which means that the default buffer size will be used.
.. function:: EncodedFile(file, data_encoding, file_encoding=None, errors='strict')
diff --git a/Lib/_pyio.py b/Lib/_pyio.py
index 01ef5b7b0bb5..b8975ff533d7 100644
--- a/Lib/_pyio.py
+++ b/Lib/_pyio.py
@@ -198,6 +198,11 @@ def open(file, mode="r", buffering=-1, encoding=None, errors=None,
raise ValueError("binary mode doesn't take an errors argument")
if binary and newline is not None:
raise ValueError("binary mode doesn't take a newline argument")
+ if binary and buffering == 1:
+ import warnings
+ warnings.warn("line buffering (buffering=1) isn't supported in binary "
+ "mode, the default buffer size will be used",
+ RuntimeWarning, 2)
raw = FileIO(file,
(creating and "x" or "") +
(reading and "r" or "") +
diff --git a/Lib/codecs.py b/Lib/codecs.py
index a70ed20f2bc7..6b028adb1d28 100644
--- a/Lib/codecs.py
+++ b/Lib/codecs.py
@@ -862,7 +862,7 @@ def __exit__(self, type, value, tb):
### Shortcuts
-def open(filename, mode='r', encoding=None, errors='strict', buffering=1):
+def open(filename, mode='r', encoding=None, errors='strict', buffering=-1):
""" Open an encoded file using the given mode and return
a wrapped version providing transparent encoding/decoding.
@@ -883,7 +883,8 @@ def open(filename, mode='r', encoding=None, errors='strict', buffering=1):
encoding error occurs.
buffering has the same meaning as for the builtin open() API.
- It defaults to line buffered.
+ It defaults to -1 which means that the default buffer size will
+ be used.
The returned wrapped file object provides an extra attribute
.encoding which allows querying the used encoding. This
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index c827113a933f..5db6f0cc24ba 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -743,12 +743,21 @@ def __init__(self, args, bufsize=-1, executable=None,
self._closed_child_pipe_fds = False
+ if self.text_mode:
+ if bufsize == 1:
+ line_buffering = True
+ # Use the default buffer size for the underlying binary streams
+ # since they don't support line buffering.
+ bufsize = -1
+ else:
+ line_buffering = False
+
try:
if p2cwrite != -1:
self.stdin = io.open(p2cwrite, 'wb', bufsize)
if self.text_mode:
self.stdin = io.TextIOWrapper(self.stdin, write_through=True,
- line_buffering=(bufsize == 1),
+ line_buffering=line_buffering,
encoding=encoding, errors=errors)
if c2pread != -1:
self.stdout = io.open(c2pread, 'rb', bufsize)
diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py
index ed0d46de6426..01e8935cc8d7 100644
--- a/Lib/test/support/__init__.py
+++ b/Lib/test/support/__init__.py
@@ -107,7 +107,8 @@
# threads
"threading_setup", "threading_cleanup", "reap_threads", "start_threads",
# miscellaneous
- "check_warnings", "check_no_resource_warning", "EnvironmentVarGuard",
+ "check_warnings", "check_no_resource_warning", "check_no_warnings",
+ "EnvironmentVarGuard",
"run_with_locale", "swap_item",
"swap_attr", "Matcher", "set_memlimit", "SuppressCrashReport", "sortdict",
"run_with_tz", "PGO", "missing_compiler_executable", "fd_count",
@@ -1252,6 +1253,30 @@ def check_warnings(*filters, **kwargs):
return _filterwarnings(filters, quiet)
+ at contextlib.contextmanager
+def check_no_warnings(testcase, message='', category=Warning, force_gc=False):
+ """Context manager to check that no warnings are emitted.
+
+ This context manager enables a given warning within its scope
+ and checks that no warnings are emitted even with that warning
+ enabled.
+
+ If force_gc is True, a garbage collection is attempted before checking
+ for warnings. This may help to catch warnings emitted when objects
+ are deleted, such as ResourceWarning.
+
+ Other keyword arguments are passed to warnings.filterwarnings().
+ """
+ with warnings.catch_warnings(record=True) as warns:
+ warnings.filterwarnings('always',
+ message=message,
+ category=category)
+ yield
+ if force_gc:
+ gc_collect()
+ testcase.assertEqual(warns, [])
+
+
@contextlib.contextmanager
def check_no_resource_warning(testcase):
"""Context manager to check that no ResourceWarning is emitted.
@@ -1266,11 +1291,8 @@ def check_no_resource_warning(testcase):
You must remove the object which may emit ResourceWarning before
the end of the context manager.
"""
- with warnings.catch_warnings(record=True) as warns:
- warnings.filterwarnings('always', category=ResourceWarning)
+ with check_no_warnings(testcase, category=ResourceWarning, force_gc=True):
yield
- gc_collect()
- testcase.assertEqual(warns, [])
class CleanImport(object):
diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py
index 5ec9bbbb1230..bc812eac3742 100644
--- a/Lib/test/test_cmd_line_script.py
+++ b/Lib/test/test_cmd_line_script.py
@@ -169,10 +169,10 @@ def test_stdin_loader(self):
@contextlib.contextmanager
def interactive_python(self, separate_stderr=False):
if separate_stderr:
- p = spawn_python('-i', bufsize=1, stderr=subprocess.PIPE)
+ p = spawn_python('-i', stderr=subprocess.PIPE)
stderr = p.stderr
else:
- p = spawn_python('-i', bufsize=1, stderr=subprocess.STDOUT)
+ p = spawn_python('-i', stderr=subprocess.STDOUT)
stderr = p.stdout
try:
# Drain stderr until prompt
diff --git a/Lib/test/test_file.py b/Lib/test/test_file.py
index f58d1dae6045..cd642e7aaf8b 100644
--- a/Lib/test/test_file.py
+++ b/Lib/test/test_file.py
@@ -169,22 +169,33 @@ def testBadModeArgument(self):
f.close()
self.fail("no error for invalid mode: %s" % bad_mode)
+ def _checkBufferSize(self, s):
+ try:
+ f = self.open(TESTFN, 'wb', s)
+ f.write(str(s).encode("ascii"))
+ f.close()
+ f.close()
+ f = self.open(TESTFN, 'rb', s)
+ d = int(f.read().decode("ascii"))
+ f.close()
+ f.close()
+ except OSError as msg:
+ self.fail('error setting buffer size %d: %s' % (s, str(msg)))
+ self.assertEqual(d, s)
+
def testSetBufferSize(self):
# make sure that explicitly setting the buffer size doesn't cause
# misbehaviour especially with repeated close() calls
- for s in (-1, 0, 1, 512):
- try:
- f = self.open(TESTFN, 'wb', s)
- f.write(str(s).encode("ascii"))
- f.close()
- f.close()
- f = self.open(TESTFN, 'rb', s)
- d = int(f.read().decode("ascii"))
- f.close()
- f.close()
- except OSError as msg:
- self.fail('error setting buffer size %d: %s' % (s, str(msg)))
- self.assertEqual(d, s)
+ for s in (-1, 0, 512):
+ with support.check_no_warnings(self,
+ message='line buffering',
+ category=RuntimeWarning):
+ self._checkBufferSize(s)
+
+ # test that attempts to use line buffering in binary mode cause
+ # a warning
+ with self.assertWarnsRegex(RuntimeWarning, 'line buffering'):
+ self._checkBufferSize(1)
def testTruncateOnWindows(self):
# SF bug <http://www.python.org/sf/801631>
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index c68b2fea858b..abd55387bdbf 100644
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -593,7 +593,7 @@ def test_large_file_ops(self):
self.large_file_ops(f)
def test_with_open(self):
- for bufsize in (0, 1, 100):
+ for bufsize in (0, 100):
f = None
with self.open(support.TESTFN, "wb", bufsize) as f:
f.write(b"xxx")
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index c56e1b632150..b0b6b06e9275 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1136,7 +1136,8 @@ 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)
+ with self.assertWarnsRegex(RuntimeWarning, 'line buffering'):
+ 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
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-50-40.bpo-32236.3RupnN.rst b/Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-50-40.bpo-32236.3RupnN.rst
new file mode 100644
index 000000000000..6fc138708c4d
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-50-40.bpo-32236.3RupnN.rst
@@ -0,0 +1,2 @@
+Warn that line buffering is not supported if :func:`open` is called with
+binary mode and ``buffering=1``.
diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c
index eedca011d2ed..965c4846bdb8 100644
--- a/Modules/_io/_iomodule.c
+++ b/Modules/_io/_iomodule.c
@@ -363,6 +363,15 @@ _io_open_impl(PyObject *module, PyObject *file, const char *mode,
goto error;
}
+ if (binary && buffering == 1) {
+ if (PyErr_WarnEx(PyExc_RuntimeWarning,
+ "line buffering (buffering=1) isn't supported in "
+ "binary mode, the default buffer size will be used",
+ 1) < 0) {
+ goto error;
+ }
+ }
+
/* Create the Raw file stream */
{
PyObject *RawIO_class = (PyObject *)&PyFileIO_Type;
More information about the Python-checkins
mailing list