From 44235041f3b957abd36d3792450c3540aa09e120 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 12 Apr 2019 17:06:47 +0200 Subject: bpo-18748: io.IOBase destructor now logs close() errors in dev mode (GH-12786) In development mode (-X dev) and in debug build, the io.IOBase destructor now logs close() exceptions. These exceptions are silent by default in release mode. --- Doc/using/cmdline.rst | 4 ++- Lib/test/test_io.py | 31 +++++++++++++++++----- .../2019-04-11-16-09-42.bpo-18748.QW7upB.rst | 3 +++ Modules/_io/iobase.c | 18 ++++++++++--- 4 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-04-11-16-09-42.bpo-18748.QW7upB.rst diff --git a/Doc/using/cmdline.rst b/Doc/using/cmdline.rst index bd3cdef..0574336 100644 --- a/Doc/using/cmdline.rst +++ b/Doc/using/cmdline.rst @@ -437,6 +437,7 @@ Miscellaneous options * Enable :ref:`asyncio debug mode `. * Set the :attr:`~sys.flags.dev_mode` attribute of :attr:`sys.flags` to ``True`` + * :class:`io.IOBase` destructor logs ``close()`` exceptions. * ``-X utf8`` enables UTF-8 mode for operating system interfaces, overriding the default locale-aware mode. ``-X utf8=0`` explicitly disables UTF-8 @@ -465,7 +466,8 @@ Miscellaneous options The ``-X importtime``, ``-X dev`` and ``-X utf8`` options. .. versionadded:: 3.8 - The ``-X pycache_prefix`` option. + The ``-X pycache_prefix`` option. The ``-X dev`` option now logs + ``close()`` exceptions in :class:`io.IOBase` destructor. Options you shouldn't use diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index d245c5d..811a446 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -67,6 +67,11 @@ MEMORY_SANITIZER = ( '--with-memory-sanitizer' in _config_args ) +# Does io.IOBase logs unhandled exceptions on calling close()? +# They are silenced by default in release build. +DESTRUCTOR_LOG_ERRORS = (hasattr(sys, "gettotalrefcount") or sys.flags.dev_mode) + + def _default_chunk_size(): """Get the default TextIOWrapper chunk size""" with open(__file__, "r", encoding="latin-1") as f: @@ -1097,9 +1102,16 @@ class CommonBufferedTests: s = s.getvalue().strip() if s: # The destructor *may* have printed an unraisable error, check it - self.assertEqual(len(s.splitlines()), 1) - self.assertTrue(s.startswith("Exception OSError: "), s) - self.assertTrue(s.endswith(" ignored"), s) + lines = s.splitlines() + if DESTRUCTOR_LOG_ERRORS: + self.assertEqual(len(lines), 5) + self.assertTrue(lines[0].startswith("Exception ignored in: "), lines) + self.assertEqual(lines[1], "Traceback (most recent call last):", lines) + self.assertEqual(lines[4], 'OSError:', lines) + else: + self.assertEqual(len(lines), 1) + self.assertTrue(lines[-1].startswith("Exception OSError: "), lines) + self.assertTrue(lines[-1].endswith(" ignored"), lines) def test_repr(self): raw = self.MockRawIO() @@ -2833,9 +2845,16 @@ class TextIOWrapperTest(unittest.TestCase): s = s.getvalue().strip() if s: # The destructor *may* have printed an unraisable error, check it - self.assertEqual(len(s.splitlines()), 1) - self.assertTrue(s.startswith("Exception OSError: "), s) - self.assertTrue(s.endswith(" ignored"), s) + lines = s.splitlines() + if DESTRUCTOR_LOG_ERRORS: + self.assertEqual(len(lines), 5) + self.assertTrue(lines[0].startswith("Exception ignored in: "), lines) + self.assertEqual(lines[1], "Traceback (most recent call last):", lines) + self.assertEqual(lines[4], 'OSError:', lines) + else: + self.assertEqual(len(lines), 1) + self.assertTrue(lines[-1].startswith("Exception OSError: "), lines) + self.assertTrue(lines[-1].endswith(" ignored"), lines) # Systematic tests of the text I/O API diff --git a/Misc/NEWS.d/next/Library/2019-04-11-16-09-42.bpo-18748.QW7upB.rst b/Misc/NEWS.d/next/Library/2019-04-11-16-09-42.bpo-18748.QW7upB.rst new file mode 100644 index 0000000..2e0cef8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-04-11-16-09-42.bpo-18748.QW7upB.rst @@ -0,0 +1,3 @@ +In development mode (:option:`-X` ``dev``) and in debug build, the +:class:`io.IOBase` destructor now logs ``close()`` exceptions. These exceptions +are silent by default in release mode. diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 9b063cd..3a8f16a 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -286,10 +286,22 @@ iobase_finalize(PyObject *self) /* Silencing I/O errors is bad, but printing spurious tracebacks is equally as bad, and potentially more frequent (because of shutdown issues). */ - if (res == NULL) - PyErr_Clear(); - else + if (res == NULL) { +#ifndef Py_DEBUG + const _PyCoreConfig *config = &_PyInterpreterState_GET_UNSAFE()->core_config; + if (config->dev_mode) { + PyErr_WriteUnraisable(self); + } + else { + PyErr_Clear(); + } +#else + PyErr_WriteUnraisable(self); +#endif + } + else { Py_DECREF(res); + } } /* Restore the saved exception. */ -- cgit v0.12