Skip to content

bpo-18748: _pyio.IOBase emits unraisable exception #13512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,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