From 1fc116ba990dfa4e588d6a757c1cb2645c76bc76 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 22 May 2019 12:18:52 +0200 Subject: [PATCH 1/3] bpo-36829: Add test.support.catch_unraisable_exception() * Copy test_exceptions.test_unraisable() to test_sys.UnraisableHookTest(). * test_exceptions.test_unraisable() uses catch_unraisable_exception(); simplify the test. test_sys now checks the exact output. * Use catch_unraisable_exception() in test_coroutines, test_exceptions, test_generators. --- Lib/test/support/__init__.py | 36 ++++++++++++++++ Lib/test/test_coroutines.py | 24 ++++++++--- Lib/test/test_exceptions.py | 26 ++++++------ Lib/test/test_generators.py | 32 +++++++-------- Lib/test/test_sys.py | 41 +++++++++++++++++++ .../2019-05-22-12-57-15.bpo-36829.e9mRWC.rst | 2 + 6 files changed, 125 insertions(+), 36 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2019-05-22-12-57-15.bpo-36829.e9mRWC.rst diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 9e60d960ab12f4..b231c79b652de1 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -3034,3 +3034,39 @@ def collision_stats(nbins, nballs): collisions = k - occupied var = dn*(dn-1)*((dn-2)/dn)**k + meanempty * (1 - meanempty) return float(collisions), float(var.sqrt()) + + +class catch_unraisable_exception: + """ + Context manager catching unraisable exception using sys.unraisablehook. + + Usage: + + try: + with support.catch_unraisable_exception() as cm: + ... + + # check the expected unraisable exception + ... + finally: + # Explicitly break reference cycle + cm = None + + The finally block is required: cm.unraisable contains a traceback object + which indirectly references the 'cm' variable, it's a reference cycle. + """ + + def __init__(self): + self.unraisable = None + self._old_hook = None + + def _hook(self, unraisable): + self.unraisable = unraisable + + def __enter__(self): + self._old_hook = sys.unraisablehook + sys.unraisablehook = self._hook + return self + + def __exit__(self, *exc_info): + sys.unraisablehook = self._old_hook diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 8443e658a620b9..06c0fed3288f70 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -2342,12 +2342,24 @@ async def corofn(): orig_wuc = warnings._warn_unawaited_coroutine try: warnings._warn_unawaited_coroutine = lambda coro: 1/0 - with support.captured_stderr() as stream: - corofn() - support.gc_collect() - self.assertIn("Exception ignored in", stream.getvalue()) - self.assertIn("ZeroDivisionError", stream.getvalue()) - self.assertIn("was never awaited", stream.getvalue()) + try: + # Note: catch_unraisable_exception() resurect the coroutine + with support.catch_unraisable_exception() as cm, \ + support.captured_stderr() as stream: + # only store repr() to avoid keeping the coroutine alive + coro = corofn() + coro_repr = repr(coro) + + # clear reference to the coroutine without awaiting for it + del coro + support.gc_collect() + + self.assertEqual(repr(cm.unraisable.object), coro_repr) + self.assertEqual(cm.unraisable.exc_type, ZeroDivisionError) + self.assertIn("was never awaited", stream.getvalue()) + finally: + # Explicitly break reference cycle + cm = None del warnings._warn_unawaited_coroutine with support.captured_stderr() as stream: diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 6ef529e2b015be..243db794294aa5 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -12,6 +12,9 @@ check_warnings, cpython_only, gc_collect, run_unittest, no_tracing, unlink, import_module, script_helper, SuppressCrashReport) +from test import support + + class NaiveException(Exception): def __init__(self, x): self.x = x @@ -1190,20 +1193,15 @@ def __del__(self): for test_class in (BrokenDel, BrokenExceptionDel): with self.subTest(test_class): obj = test_class() - with captured_stderr() as stderr: - del obj - report = stderr.getvalue() - self.assertIn("Exception ignored", report) - self.assertIn(test_class.__del__.__qualname__, report) - self.assertIn("test_exceptions.py", report) - self.assertIn("raise exc", report) - if test_class is BrokenExceptionDel: - self.assertIn("BrokenStrException", report) - self.assertIn("", report) - else: - self.assertIn("ValueError", report) - self.assertIn("del is broken", report) - self.assertTrue(report.endswith("\n")) + try: + with support.catch_unraisable_exception() as cm: + del obj + + self.assertEqual(cm.unraisable.object, test_class.__del__) + self.assertIsNotNone(cm.unraisable.exc_traceback) + finally: + # Explicitly break reference cycle + cm = None def test_unhandled(self): # Check for sensible reporting of unhandled exceptions diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 320793c7dab69b..60b7dd27a01fee 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -2156,25 +2156,25 @@ def printsolution(self, x): printing warnings and to doublecheck that we actually tested what we wanted to test. ->>> import sys, io ->>> old = sys.stderr +>>> from test import support +>>> class Leaker: +... def __del__(self): +... def invoke(message): +... raise RuntimeError(message) +... invoke("del failed") +... >>> try: -... sys.stderr = io.StringIO() -... class Leaker: -... def __del__(self): -... def invoke(message): -... raise RuntimeError(message) -... invoke("test") +... with support.catch_unraisable_exception() as cm: +... l = Leaker() +... del l ... -... l = Leaker() -... del l -... err = sys.stderr.getvalue().strip() -... "Exception ignored in" in err -... "RuntimeError: test" in err -... "Traceback" in err -... "in invoke" in err +... Leaker.__del__ == cm.unraisable.object +... cm.unraisable.exc_type == RuntimeError +... str(cm.unraisable.exc_value) == "del failed" +... cm.unraisable.exc_traceback is not None ... finally: -... sys.stderr = old +... # Explicitly break reference cycle +... cm = None True True True diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 2b358ca0466e6e..67a952d9b4544c 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -909,6 +909,47 @@ def test_original_unraisablehook(self): self.assertIn('Traceback (most recent call last):\n', err) self.assertIn('ValueError: 42\n', err) + def test_original_unraisablehook_err(self): + # bpo-22836: PyErr_WriteUnraisable() should give sensible reports + class BrokenDel: + def __del__(self): + exc = ValueError("del is broken") + # The following line is included in the traceback report: + raise exc + + class BrokenStrException(Exception): + def __str__(self): + raise Exception("str() is broken") + + class BrokenExceptionDel: + def __del__(self): + exc = BrokenStrException() + # The following line is included in the traceback report: + raise exc + + for test_class in (BrokenDel, BrokenExceptionDel): + with self.subTest(test_class): + obj = test_class() + with test.support.captured_stderr() as stderr, \ + test.support.swap_attr(sys, 'unraisablehook', + sys.__unraisablehook__): + # Trigger obj.__del__() + del obj + + report = stderr.getvalue() + self.assertIn("Exception ignored", report) + self.assertIn(test_class.__del__.__qualname__, report) + self.assertIn("test_sys.py", report) + self.assertIn("raise exc", report) + if test_class is BrokenExceptionDel: + self.assertIn("BrokenStrException", report) + self.assertIn("", report) + else: + self.assertIn("ValueError", report) + self.assertIn("del is broken", report) + self.assertTrue(report.endswith("\n")) + + def test_original_unraisablehook_wrong_type(self): exc = ValueError(42) with test.support.swap_attr(sys, 'unraisablehook', diff --git a/Misc/NEWS.d/next/Tests/2019-05-22-12-57-15.bpo-36829.e9mRWC.rst b/Misc/NEWS.d/next/Tests/2019-05-22-12-57-15.bpo-36829.e9mRWC.rst new file mode 100644 index 00000000000000..4ab342b8a2b364 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2019-05-22-12-57-15.bpo-36829.e9mRWC.rst @@ -0,0 +1,2 @@ +Add :func:`test.support.catch_unraisable_exception`: context manager +catching unraisable exception using :func:`sys.unraisablehook`. From 1ffe98dc32e90d45b58210a91017f67bb4897e46 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 22 May 2019 18:28:40 +0200 Subject: [PATCH 2/3] catch_unraisable_exception.__exit__ clears unraisable Avoid the need for try/finally: __exit__ clears unraisable to break the reference cycle. --- Lib/test/support/__init__.py | 14 +++++++------- Lib/test/test_coroutines.py | 23 +++++++++-------------- Lib/test/test_exceptions.py | 11 ++++------- Lib/test/test_generators.py | 10 +++------- 4 files changed, 23 insertions(+), 35 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index b231c79b652de1..e32400aa35cd4d 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -3042,15 +3042,13 @@ class catch_unraisable_exception: Usage: - try: - with support.catch_unraisable_exception() as cm: - ... + with support.catch_unraisable_exception() as cm: + ... - # check the expected unraisable exception + # check the expected unraisable exception: use cm.unraisable ... - finally: - # Explicitly break reference cycle - cm = None + + # cm.unraisable is None here (to break a reference cycle) The finally block is required: cm.unraisable contains a traceback object which indirectly references the 'cm' variable, it's a reference cycle. @@ -3069,4 +3067,6 @@ def __enter__(self): return self def __exit__(self, *exc_info): + # Clear the unraisable exception to explicitly break a reference cycle + self.unraisable = None sys.unraisablehook = self._old_hook diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 06c0fed3288f70..036f13fa50e9ae 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -2342,24 +2342,19 @@ async def corofn(): orig_wuc = warnings._warn_unawaited_coroutine try: warnings._warn_unawaited_coroutine = lambda coro: 1/0 - try: - # Note: catch_unraisable_exception() resurect the coroutine - with support.catch_unraisable_exception() as cm, \ - support.captured_stderr() as stream: - # only store repr() to avoid keeping the coroutine alive - coro = corofn() - coro_repr = repr(coro) - - # clear reference to the coroutine without awaiting for it - del coro - support.gc_collect() + with support.catch_unraisable_exception() as cm, \ + support.captured_stderr() as stream: + # only store repr() to avoid keeping the coroutine alive + coro = corofn() + coro_repr = repr(coro) + + # clear reference to the coroutine without awaiting for it + del coro + support.gc_collect() self.assertEqual(repr(cm.unraisable.object), coro_repr) self.assertEqual(cm.unraisable.exc_type, ZeroDivisionError) self.assertIn("was never awaited", stream.getvalue()) - finally: - # Explicitly break reference cycle - cm = None del warnings._warn_unawaited_coroutine with support.captured_stderr() as stream: diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 243db794294aa5..fe78d410531792 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1193,15 +1193,12 @@ def __del__(self): for test_class in (BrokenDel, BrokenExceptionDel): with self.subTest(test_class): obj = test_class() - try: - with support.catch_unraisable_exception() as cm: - del obj + with support.catch_unraisable_exception() as cm: + del obj - self.assertEqual(cm.unraisable.object, test_class.__del__) + self.assertEqual(cm.unraisable.object, + test_class.__del__) self.assertIsNotNone(cm.unraisable.exc_traceback) - finally: - # Explicitly break reference cycle - cm = None def test_unhandled(self): # Check for sensible reporting of unhandled exceptions diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 60b7dd27a01fee..ffd40b833b08fe 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -2163,18 +2163,14 @@ def printsolution(self, x): ... raise RuntimeError(message) ... invoke("del failed") ... ->>> try: -... with support.catch_unraisable_exception() as cm: -... l = Leaker() -... del l +>>> with support.catch_unraisable_exception() as cm: +... l = Leaker() +... del l ... ... Leaker.__del__ == cm.unraisable.object ... cm.unraisable.exc_type == RuntimeError ... str(cm.unraisable.exc_value) == "del failed" ... cm.unraisable.exc_traceback is not None -... finally: -... # Explicitly break reference cycle -... cm = None True True True From 5c1bf390e3a6a6842a21567d9074ebee5b1a2c28 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 22 May 2019 23:08:10 +0200 Subject: [PATCH 3/3] Cleanup --- Lib/test/support/__init__.py | 3 --- Lib/test/test_exceptions.py | 19 +++++-------------- Lib/test/test_generators.py | 2 +- 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index e32400aa35cd4d..2fe9d9dc80998f 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -3049,9 +3049,6 @@ class catch_unraisable_exception: ... # cm.unraisable is None here (to break a reference cycle) - - The finally block is required: cm.unraisable contains a traceback object - which indirectly references the 'cm' variable, it's a reference cycle. """ def __init__(self): diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index fe78d410531792..d7e11d2d30a87c 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1184,21 +1184,12 @@ def __del__(self): # The following line is included in the traceback report: raise exc - class BrokenExceptionDel: - def __del__(self): - exc = BrokenStrException() - # The following line is included in the traceback report: - raise exc - - for test_class in (BrokenDel, BrokenExceptionDel): - with self.subTest(test_class): - obj = test_class() - with support.catch_unraisable_exception() as cm: - del obj + obj = BrokenDel() + with support.catch_unraisable_exception() as cm: + del obj - self.assertEqual(cm.unraisable.object, - test_class.__del__) - self.assertIsNotNone(cm.unraisable.exc_traceback) + self.assertEqual(cm.unraisable.object, BrokenDel.__del__) + self.assertIsNotNone(cm.unraisable.exc_traceback) def test_unhandled(self): # Check for sensible reporting of unhandled exceptions diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index ffd40b833b08fe..7f1472fa03ac30 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -2167,7 +2167,7 @@ def printsolution(self, x): ... l = Leaker() ... del l ... -... Leaker.__del__ == cm.unraisable.object +... cm.unraisable.object == Leaker.__del__ ... cm.unraisable.exc_type == RuntimeError ... str(cm.unraisable.exc_value) == "del failed" ... cm.unraisable.exc_traceback is not None