From 2b0a0d841786bb394b6f094c442e95997a4967d7 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Tue, 12 Dec 2017 21:20:15 +0300 Subject: [PATCH 01/12] bpo-32236: Don't allow buffering=1 in open() for binary mode 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). Raise ValueError on attempts to use line buffering in binary mode to prevent possible bugs and expose existing buggy code. --- Lib/_pyio.py | 2 ++ Lib/codecs.py | 2 +- Lib/subprocess.py | 10 +++++++++- Lib/test/test_cmd_line_script.py | 4 ++-- Lib/test/test_file.py | 6 +++++- Lib/test/test_io.py | 2 +- Lib/test/test_subprocess.py | 7 +++---- Modules/_io/_iomodule.c | 6 ++++++ 8 files changed, 29 insertions(+), 10 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index adf5d0ecbf69b0..6ed00bb6ffb44d 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -198,6 +198,8 @@ 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: + raise ValueError("line buffering is not supported in binary mode") raw = FileIO(file, (creating and "x" or "") + (reading and "r" or "") + diff --git a/Lib/codecs.py b/Lib/codecs.py index a70ed20f2bc794..85392a220fd8ee 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. diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 35bfddde4e92f1..133588c09438bb 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -721,12 +721,20 @@ def __init__(self, args, bufsize=-1, executable=None, self._closed_child_pipe_fds = False + if self.text_mode: + if bufsize == 1: + line_buffering = True + # line buffering is not supported in binary mode + 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/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index 0d0bcd784d2688..3e1fe3759010c5 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -177,10 +177,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 65be30f0b2edb5..17e0028338fc62 100644 --- a/Lib/test/test_file.py +++ b/Lib/test/test_file.py @@ -167,7 +167,7 @@ def testBadModeArgument(self): 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): + for s in (-1, 0, 512): try: f = self.open(TESTFN, 'wb', s) f.write(str(s).encode("ascii")) @@ -181,6 +181,10 @@ def testSetBufferSize(self): self.fail('error setting buffer size %d: %s' % (s, str(msg))) self.assertEqual(d, s) + # test that attempts to use line buffering in binary mode are rejected + self.assertRaises(ValueError, self.open, TESTFN, 'wb', 1) + self.assertRaises(ValueError, self.open, TESTFN, 'rb', 1) + def testTruncateOnWindows(self): # SF bug # "file.truncate fault on windows" diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 2ac2e17a03e62e..02c6691d5c9ab2 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -592,7 +592,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 fff1b0db1051b6..0558e8013a26d6 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1134,10 +1134,9 @@ def test_bufsize_equal_one_text_mode(self): 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) + # bufsize=1 should not be accepted in binary mode. + self.assertRaises(ValueError, self._test_bufsize_equal_one, + b'', b'', universal_newlines=False) def test_leaking_fds_on_error(self): # see bug #5179: Popen leaks file descriptors to PIPEs if diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index 5db44f970d22ba..a9da7ccefe6a05 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -362,6 +362,12 @@ _io_open_impl(PyObject *module, PyObject *file, const char *mode, goto error; } + if (binary && buffering == 1) { + PyErr_SetString(PyExc_ValueError, + "line buffering is not supported in binary mode"); + goto error; + } + /* Create the Raw file stream */ { PyObject *RawIO_class = (PyObject *)&PyFileIO_Type; From 888ee4dfa809c17fa04dd088b5c2395042781357 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Mon, 18 Dec 2017 18:59:46 +0300 Subject: [PATCH 02/12] bpo-32236: Issue RuntimeWarning instead of raising ValueError --- Lib/_pyio.py | 4 +++- Lib/test/test_file.py | 35 ++++++++++++++++++++--------------- Lib/test/test_io.py | 14 +++++++++++--- Lib/test/test_subprocess.py | 9 ++++++--- Modules/_io/_iomodule.c | 7 ++++--- 5 files changed, 44 insertions(+), 25 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 6ed00bb6ffb44d..5886727f8b821b 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -199,7 +199,9 @@ def open(file, mode="r", buffering=-1, encoding=None, errors=None, if binary and newline is not None: raise ValueError("binary mode doesn't take a newline argument") if binary and buffering == 1: - raise ValueError("line buffering is not supported in binary mode") + import warnings + warnings.warn("line buffering is not supported in binary mode", + RuntimeWarning, 2) raw = FileIO(file, (creating and "x" or "") + (reading and "r" or "") + diff --git a/Lib/test/test_file.py b/Lib/test/test_file.py index 17e0028338fc62..818fdec07b5bc3 100644 --- a/Lib/test/test_file.py +++ b/Lib/test/test_file.py @@ -164,26 +164,31 @@ 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, 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) + self._checkBufferSize(s) - # test that attempts to use line buffering in binary mode are rejected - self.assertRaises(ValueError, self.open, TESTFN, 'wb', 1) - self.assertRaises(ValueError, self.open, TESTFN, 'rb', 1) + # test that attempts to use line buffering in binary mode cause + # a warning + warn_msg = 'line buffering is not supported in binary mode' + with self.assertWarns(RuntimeWarning, msg=warn_msg): + self._checkBufferSize(1) def testTruncateOnWindows(self): # SF bug diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 02c6691d5c9ab2..903af8663019bc 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -591,15 +591,23 @@ def test_large_file_ops(self): with self.open(support.TESTFN, "w+b") as f: self.large_file_ops(f) + def _checked_open(self, path, mode, bufsize, under_check=False): + if not under_check and bufsize == 1: + warn_msg = 'line buffering is not supported in binary mode' + with self.assertWarns(RuntimeWarning, msg=warn_msg): + return self._checked_open(path, mode, 1, True) + else: + return self.open(path, mode, bufsize) + def test_with_open(self): - for bufsize in (0, 100): + for bufsize in (0, 1, 100): f = None - with self.open(support.TESTFN, "wb", bufsize) as f: + with self._checked_open(support.TESTFN, "wb", bufsize) as f: f.write(b"xxx") self.assertEqual(f.closed, True) f = None try: - with self.open(support.TESTFN, "wb", bufsize) as f: + with self._checked_open(support.TESTFN, "wb", bufsize) as f: 1/0 except ZeroDivisionError: self.assertEqual(f.closed, True) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 0558e8013a26d6..b3452e32505553 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1134,9 +1134,12 @@ def test_bufsize_equal_one_text_mode(self): self._test_bufsize_equal_one(line, line, universal_newlines=True) def test_bufsize_equal_one_binary_mode(self): - # bufsize=1 should not be accepted in binary mode. - self.assertRaises(ValueError, self._test_bufsize_equal_one, - b'', b'', universal_newlines=False) + # 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 + warn_msg = 'line buffering is not supported in binary mode' + with self.assertWarns(RuntimeWarning, msg=warn_msg): + 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/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index a9da7ccefe6a05..643ad5b65bb065 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -363,9 +363,10 @@ _io_open_impl(PyObject *module, PyObject *file, const char *mode, } if (binary && buffering == 1) { - PyErr_SetString(PyExc_ValueError, - "line buffering is not supported in binary mode"); - goto error; + if (PyErr_WarnEx(PyExc_RuntimeWarning, + "line buffering is not supported in binary mode", + 1) < 0) + goto error; } /* Create the Raw file stream */ From 0deefe1e8a13116e9b14dff8183dc05fbfa13db4 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Tue, 11 Sep 2018 21:07:41 +0300 Subject: [PATCH 03/12] Clarify warning message --- Lib/_pyio.py | 4 ++-- Modules/_io/_iomodule.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 5886727f8b821b..9921e5d3a2839f 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -200,8 +200,8 @@ def open(file, mode="r", buffering=-1, encoding=None, errors=None, raise ValueError("binary mode doesn't take a newline argument") if binary and buffering == 1: import warnings - warnings.warn("line buffering is not supported in binary mode", - RuntimeWarning, 2) + warnings.warn("line buffering (buffering=1) isn't supported in binary " + "mode, file will be fully buffered", RuntimeWarning, 2) raw = FileIO(file, (creating and "x" or "") + (reading and "r" or "") + diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index 643ad5b65bb065..11d156a9d543eb 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -364,8 +364,8 @@ _io_open_impl(PyObject *module, PyObject *file, const char *mode, if (binary && buffering == 1) { if (PyErr_WarnEx(PyExc_RuntimeWarning, - "line buffering is not supported in binary mode", - 1) < 0) + "line buffering (buffering=1) isn't supported in " + "binary mode, file will be fully buffered", 1) < 0) goto error; } From 42a43c10b0de5061834041fcc545fb1190774b7b Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Tue, 11 Sep 2018 23:06:40 +0300 Subject: [PATCH 04/12] Remove RuntimeWarning test from test_io.py --- Lib/test/test_io.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 903af8663019bc..02c6691d5c9ab2 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -591,23 +591,15 @@ def test_large_file_ops(self): with self.open(support.TESTFN, "w+b") as f: self.large_file_ops(f) - def _checked_open(self, path, mode, bufsize, under_check=False): - if not under_check and bufsize == 1: - warn_msg = 'line buffering is not supported in binary mode' - with self.assertWarns(RuntimeWarning, msg=warn_msg): - return self._checked_open(path, mode, 1, True) - else: - return self.open(path, mode, bufsize) - def test_with_open(self): - for bufsize in (0, 1, 100): + for bufsize in (0, 100): f = None - with self._checked_open(support.TESTFN, "wb", bufsize) as f: + with self.open(support.TESTFN, "wb", bufsize) as f: f.write(b"xxx") self.assertEqual(f.closed, True) f = None try: - with self._checked_open(support.TESTFN, "wb", bufsize) as f: + with self.open(support.TESTFN, "wb", bufsize) as f: 1/0 except ZeroDivisionError: self.assertEqual(f.closed, True) From 164470a41bec21ea7cbe22dd0217b537e238810b Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Tue, 11 Sep 2018 23:36:12 +0300 Subject: [PATCH 05/12] Fix check for warning message --- Lib/test/test_file.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_file.py b/Lib/test/test_file.py index 818fdec07b5bc3..0d62e5b72711eb 100644 --- a/Lib/test/test_file.py +++ b/Lib/test/test_file.py @@ -186,8 +186,7 @@ def testSetBufferSize(self): # test that attempts to use line buffering in binary mode cause # a warning - warn_msg = 'line buffering is not supported in binary mode' - with self.assertWarns(RuntimeWarning, msg=warn_msg): + with self.assertWarnsRegex(RuntimeWarning, 'line buffering'): self._checkBufferSize(1) def testTruncateOnWindows(self): From 54c12b2884933e6805ba61f305785349dfe67173 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Wed, 12 Sep 2018 00:40:32 +0300 Subject: [PATCH 06/12] Check that no warning is emitted when buffering != 1 --- Lib/test/support/__init__.py | 25 +++++++++++++++++++++---- Lib/test/test_file.py | 5 ++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 22868d4ba1a389..2ab8169493f5b8 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -1208,6 +1208,26 @@ def check_warnings(*filters, **kwargs): return _filterwarnings(filters, quiet) +@contextlib.contextmanager +def check_no_warning(testcase, message='', category=Warning, force_gc=False): + """Context manager to check that no matching warning is emitted. + + If force_gc is True, attempt a garbage collection 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. @@ -1222,11 +1242,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_warning(testcase, category=ResourceWarning, force_gc=True): yield - gc_collect() - testcase.assertEqual(warns, []) class CleanImport(object): diff --git a/Lib/test/test_file.py b/Lib/test/test_file.py index 0d62e5b72711eb..0c3f3e21d685c1 100644 --- a/Lib/test/test_file.py +++ b/Lib/test/test_file.py @@ -182,7 +182,10 @@ 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, 512): - self._checkBufferSize(s) + with support.check_no_warning(self, + message='line buffering', + category=RuntimeWarning): + self._checkBufferSize(s) # test that attempts to use line buffering in binary mode cause # a warning From 3fe49c4f113bb2a70c27bee8a833aa862545ba75 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Tue, 11 Sep 2018 23:51:36 +0300 Subject: [PATCH 07/12] Add NEWS entry --- .../Core and Builtins/2018-09-11-23-50-40.bpo-32236.3RupnN.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-09-11-23-50-40.bpo-32236.3RupnN.rst 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 00000000000000..6fc138708c4dfc --- /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``. From aff54cf8c05bc46d9a834a660129cbb16dad2920 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Wed, 12 Sep 2018 01:13:00 +0300 Subject: [PATCH 08/12] Fix warning message check in test_subprocess.py --- Lib/test/test_subprocess.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index b3452e32505553..d20a183f175db5 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1137,8 +1137,7 @@ 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 - warn_msg = 'line buffering is not supported in binary mode' - with self.assertWarns(RuntimeWarning, msg=warn_msg): + with self.assertWarnsRegex(RuntimeWarning, 'line buffering'): self._test_bufsize_equal_one(line, b'', universal_newlines=False) def test_leaking_fds_on_error(self): From cec8cd7b3efdce65ef35acfec869d47ed18dfdba Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Wed, 12 Sep 2018 02:23:50 +0300 Subject: [PATCH 09/12] Update docs for codecs.open() --- Doc/library/codecs.rst | 6 +++--- Lib/codecs.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Doc/library/codecs.rst b/Doc/library/codecs.rst index 6e249ecf2b1af4..e2275ca91c3a0d 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/codecs.py b/Lib/codecs.py index 85392a220fd8ee..6b028adb1d28e6 100644 --- a/Lib/codecs.py +++ b/Lib/codecs.py @@ -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 From 64dfd42ba4860ffcd38d0ca305ad82e7b6a4c8e0 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Wed, 12 Sep 2018 03:55:40 +0300 Subject: [PATCH 10/12] Fix check_no_warnings() docs to reflect what it actually does My previous understanding of behavior of check_no_resource_warning(), which check_no_warnings() is based on, was incorrect. --- Lib/test/support/__init__.py | 15 ++++++++++----- Lib/test/test_file.py | 6 +++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 2ab8169493f5b8..73816fa91d2bac 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -101,7 +101,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", @@ -1209,10 +1210,14 @@ def check_warnings(*filters, **kwargs): @contextlib.contextmanager -def check_no_warning(testcase, message='', category=Warning, force_gc=False): - """Context manager to check that no matching warning is emitted. +def check_no_warnings(testcase, message='', category=Warning, force_gc=False): + """Context manager to check that no warnings are emitted. - If force_gc is True, attempt a garbage collection before checking + 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. @@ -1242,7 +1247,7 @@ def check_no_resource_warning(testcase): You must remove the object which may emit ResourceWarning before the end of the context manager. """ - with check_no_warning(testcase, category=ResourceWarning, force_gc=True): + with check_no_warnings(testcase, category=ResourceWarning, force_gc=True): yield diff --git a/Lib/test/test_file.py b/Lib/test/test_file.py index 0c3f3e21d685c1..c84ac7f4933953 100644 --- a/Lib/test/test_file.py +++ b/Lib/test/test_file.py @@ -182,9 +182,9 @@ 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, 512): - with support.check_no_warning(self, - message='line buffering', - category=RuntimeWarning): + 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 From 2538bde85783e0e619f0e5b827a2075dcbc11400 Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Fri, 19 Oct 2018 00:28:55 +0300 Subject: [PATCH 11/12] Clarify warning message --- Lib/_pyio.py | 3 ++- Modules/_io/_iomodule.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 9921e5d3a2839f..70bbe051b126f3 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -201,7 +201,8 @@ def open(file, mode="r", buffering=-1, encoding=None, errors=None, if binary and buffering == 1: import warnings warnings.warn("line buffering (buffering=1) isn't supported in binary " - "mode, file will be fully buffered", RuntimeWarning, 2) + "mode, the default buffer size will be used", + RuntimeWarning, 2) raw = FileIO(file, (creating and "x" or "") + (reading and "r" or "") + diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index 11d156a9d543eb..11958a4ff40d67 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -365,8 +365,10 @@ _io_open_impl(PyObject *module, PyObject *file, const char *mode, if (binary && buffering == 1) { if (PyErr_WarnEx(PyExc_RuntimeWarning, "line buffering (buffering=1) isn't supported in " - "binary mode, file will be fully buffered", 1) < 0) + "binary mode, the default buffer size will be used", + 1) < 0) { goto error; + } } /* Create the Raw file stream */ From 250adb44732b6c1d98250ba2a9c9c5a103d249cf Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Fri, 19 Oct 2018 00:43:18 +0300 Subject: [PATCH 12/12] Clarify a comment --- Lib/subprocess.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 133588c09438bb..0c59038e86a807 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -724,7 +724,8 @@ def __init__(self, args, bufsize=-1, executable=None, if self.text_mode: if bufsize == 1: line_buffering = True - # line buffering is not supported in binary mode + # Use the default buffer size for the underlying binary streams + # since they don't support line buffering. bufsize = -1 else: line_buffering = False