Skip to content

Commit

Permalink
bpo-18748: _pyio.IOBase emits unraisable exception (GH-13512)
Browse files Browse the repository at this point in the history
In development (-X dev) mode and in a debug build, IOBase finalizer
of the _pyio module now logs the exception if the close() method
fails. The exception is ignored silently by default in release build.

test_io: test_error_through_destructor() now uses
support.catch_unraisable_exception() rather than capturing stderr.
  • Loading branch information
vstinner authored May 23, 2019
1 parent 0a8e572 commit bc2aa81
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 45 deletions.
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,15 @@ for :func:`property`, :func:`classmethod`, and :func:`staticmethod`::
self.bit_rate = round(bit_rate / 1000.0, 1)
self.duration = ceil(duration)

io
--

In development mode (:option:`-X` ``env``) and in debug build, the
:class:`io.IOBase` finalizer now logs the exception if the ``close()`` method
fails. The exception is ignored silently by default in release build.
(Contributed by Victor Stinner in :issue:`18748`.)


gc
--

Expand Down
23 changes: 15 additions & 8 deletions Lib/_pyio.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
# 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)


def open(file, mode="r", buffering=-1, encoding=None, errors=None,
newline=None, closefd=True, opener=None):
Expand Down Expand Up @@ -378,15 +382,18 @@ def close(self):

def __del__(self):
"""Destructor. Calls close()."""
# 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:
if _IOBASE_EMITS_UNRAISABLE:
self.close()
except:
pass
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

### Inquiries ###

Expand Down
64 changes: 27 additions & 37 deletions Lib/test/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ class EmptyStruct(ctypes.Structure):
'--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)
# 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)


def _default_chunk_size():
Expand Down Expand Up @@ -1098,23 +1098,18 @@ def test_error_through_destructor(self):
# Test that the exception state is not modified by a destructor,
# even if close() fails.
rawio = self.CloseFailureIO()
def f():
self.tp(rawio).xyzzy
with support.captured_output("stderr") as s:
self.assertRaises(AttributeError, f)
s = s.getvalue().strip()
if s:
# The destructor *may* have printed an unraisable error, check it
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)
try:
with support.catch_unraisable_exception() as cm:
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)
finally:
# Explicitly break reference cycle
cm = None

def test_repr(self):
raw = self.MockRawIO()
Expand Down Expand Up @@ -2859,23 +2854,18 @@ def test_error_through_destructor(self):
# Test that the exception state is not modified by a destructor,
# even if close() fails.
rawio = self.CloseFailureIO()
def f():
self.TextIOWrapper(rawio).xyzzy
with support.captured_output("stderr") as s:
self.assertRaises(AttributeError, f)
s = s.getvalue().strip()
if s:
# The destructor *may* have printed an unraisable error, check it
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)
try:
with support.catch_unraisable_exception() as cm:
with self.assertRaises(AttributeError):
self.TextIOWrapper(rawio).xyzzy

if not IOBASE_EMITS_UNRAISABLE:
self.assertIsNone(cm.unraisable)
elif cm.unraisable is not None:
self.assertEqual(cm.unraisable.exc_type, OSError)
finally:
# Explicitly break reference cycle
cm = None

# Systematic tests of the text I/O API

Expand Down

0 comments on commit bc2aa81

Please sign in to comment.