https://github.com/python/cpython/commit/58a2e0981642dcddf49daa776ff68a43d34... commit: 58a2e0981642dcddf49daa776ff68a43d3498cee branch: main author: Victor Stinner <vstinner@python.org> committer: vstinner <vstinner@python.org> date: 2023-05-31T11:41:19Z summary: gh-62948: IOBase finalizer logs close() errors (#105104) files: A Misc/NEWS.d/next/Library/2023-05-30-18-45-02.gh-issue-62948.1-5wMR.rst M Doc/whatsnew/3.13.rst M Lib/_pyio.py M Lib/test/test_io.py M Modules/_io/iobase.c diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 7e29ed306c2d7..0fd82d6188b32 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -87,6 +87,15 @@ New Modules Improved Modules ================ +io +-- + +The :class:`io.IOBase` finalizer now logs the ``close()`` method errors with +:data:`sys.unraisablehook`. Previously, errors were ignored silently by default, +and only logged in :ref:`Python Development Mode <devmode>` or on :ref:`Python +built on debug mode <debug-build>`. +(Contributed by Victor Stinner in :gh:`62948`.) + pathlib ------- diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 7f247ff47c9e6..32698abac78d2 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -33,11 +33,8 @@ # Rebind for compatibility BlockingIOError = BlockingIOError -# Does io.IOBase finalizer log the exception if the close() method fails? -# The exception is ignored silently by default in release build. -_IOBASE_EMITS_UNRAISABLE = (hasattr(sys, "gettotalrefcount") or sys.flags.dev_mode) # Does open() check its 'errors' argument? -_CHECK_ERRORS = _IOBASE_EMITS_UNRAISABLE +_CHECK_ERRORS = (hasattr(sys, "gettotalrefcount") or sys.flags.dev_mode) def text_encoding(encoding, stacklevel=2): @@ -416,18 +413,9 @@ def __del__(self): if closed: return - if _IOBASE_EMITS_UNRAISABLE: - self.close() - else: - # The try/except block is in case this is called at program - # exit time, when it's possible that globals have already been - # deleted, and then the close() call might fail. Since - # there's nothing we can do about such failures and they annoy - # the end users, we suppress the traceback. - try: - self.close() - except: - pass + # If close() fails, the caller logs the exception with + # sys.unraisablehook. close() must be called at the end at __del__(). + self.close() ### Inquiries ### diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index cc16804fe2182..26ae40d93c84e 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -66,10 +66,6 @@ def byteslike(*pos, **kw): class EmptyStruct(ctypes.Structure): pass -# Does io.IOBase finalizer log the exception if the close() method fails? -# The exception is ignored silently by default in release build. -IOBASE_EMITS_UNRAISABLE = (support.Py_DEBUG or sys.flags.dev_mode) - def _default_chunk_size(): """Get the default TextIOWrapper chunk size""" @@ -1218,10 +1214,7 @@ def test_error_through_destructor(self): with self.assertRaises(AttributeError): self.tp(rawio).xyzzy - if not IOBASE_EMITS_UNRAISABLE: - self.assertIsNone(cm.unraisable) - elif cm.unraisable is not None: - self.assertEqual(cm.unraisable.exc_type, OSError) + self.assertEqual(cm.unraisable.exc_type, OSError) def test_repr(self): raw = self.MockRawIO() @@ -3022,10 +3015,7 @@ def test_error_through_destructor(self): with self.assertRaises(AttributeError): self.TextIOWrapper(rawio, encoding="utf-8").xyzzy - if not IOBASE_EMITS_UNRAISABLE: - self.assertIsNone(cm.unraisable) - elif cm.unraisable is not None: - self.assertEqual(cm.unraisable.exc_type, OSError) + self.assertEqual(cm.unraisable.exc_type, OSError) # Systematic tests of the text I/O API diff --git a/Misc/NEWS.d/next/Library/2023-05-30-18-45-02.gh-issue-62948.1-5wMR.rst b/Misc/NEWS.d/next/Library/2023-05-30-18-45-02.gh-issue-62948.1-5wMR.rst new file mode 100644 index 0000000000000..d6ba989329bce --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-05-30-18-45-02.gh-issue-62948.1-5wMR.rst @@ -0,0 +1,4 @@ +The :class:`io.IOBase` finalizer now logs the ``close()`` method errors with +:data:`sys.unraisablehook`. Previously, errors were ignored silently by default, +and only logged in :ref:`Python Development Mode <devmode>` or on +:ref:`Python built on debug mode <debug-build>`. Patch by Victor Stinner. diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index bcb498d9c5b5d..f98e75ce2d1ed 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -319,20 +319,8 @@ iobase_finalize(PyObject *self) if (PyObject_SetAttr(self, &_Py_ID(_finalizing), Py_True)) PyErr_Clear(); res = PyObject_CallMethodNoArgs((PyObject *)self, &_Py_ID(close)); - /* 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) { -#ifndef Py_DEBUG - if (_Py_GetConfig()->dev_mode) { - PyErr_WriteUnraisable(self); - } - else { - PyErr_Clear(); - } -#else PyErr_WriteUnraisable(self); -#endif } else { Py_DECREF(res);