Skip to content

Commit

Permalink
gh-62948: IOBase finalizer logs close() errors (#105104)
Browse files Browse the repository at this point in the history
  • Loading branch information
vstinner authored May 31, 2023
1 parent 85e5d03 commit 58a2e09
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 40 deletions.
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------

Expand Down
20 changes: 4 additions & 16 deletions Lib/_pyio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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 ###

Expand Down
14 changes: 2 additions & 12 deletions Lib/test/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 0 additions & 12 deletions Modules/_io/iobase.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 58a2e09

Please sign in to comment.