From 47a866244e2a8e794327a0ef1e80a51e89f44ca3 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 31 Jan 2025 15:42:54 +0100 Subject: [PATCH 1/5] gh-129354: Use PyErr_FormatUnraisable() function Replace PyErr_WriteUnraisable() with PyErr_FormatUnraisable(). Update tests: * test_coroutines * test_exceptions * test_generators * test_struct --- Lib/test/test_coroutines.py | 2 -- Lib/test/test_exceptions.py | 1 - Lib/test/test_generators.py | 2 -- Lib/test/test_struct.py | 2 +- Modules/_io/fileio.c | 6 ++++-- Modules/_io/iobase.c | 3 ++- Modules/socketmodule.c | 3 ++- Objects/genobject.c | 12 ++++++++---- Objects/typeobject.c | 9 ++++++--- Python/_warnings.c | 10 +++++++--- 10 files changed, 30 insertions(+), 20 deletions(-) diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 840043d5271224..50e3f60dde0bf4 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -2137,7 +2137,6 @@ async def func(): pass support.gc_collect() self.assertIn("was never awaited", str(cm.unraisable.exc_value)) - self.assertEqual(repr(cm.unraisable.object), coro_repr) def test_for_assign_raising_stop_async_iteration(self): class BadTarget: @@ -2414,7 +2413,6 @@ async def corofn(): del coro support.gc_collect() - self.assertEqual(repr(cm.unraisable.object), coro_repr) self.assertEqual(cm.unraisable.exc_type, ZeroDivisionError) del warnings._warn_unawaited_coroutine diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 2d324827451b54..ad6ff6714d2c13 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1681,7 +1681,6 @@ def __del__(self): del obj gc_collect() # For PyPy or other GCs. - self.assertEqual(cm.unraisable.object, BrokenDel.__del__) self.assertIsNotNone(cm.unraisable.exc_traceback) def test_unhandled(self): diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index b6985054c33d10..46b1988b70b51e 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -2779,14 +2779,12 @@ def printsolution(self, x): ... l = Leaker() ... del l ... -... cm.unraisable.object == Leaker.__del__ ... cm.unraisable.exc_type == RuntimeError ... str(cm.unraisable.exc_value) == "del failed" ... cm.unraisable.exc_traceback is not None True True True -True These refleak tests should perhaps be in a testfile of their own, diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 5fee9fbb92acf4..b99391e482ff70 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -694,7 +694,7 @@ def __del__(self): rc, stdout, stderr = assert_python_ok("-c", code) self.assertEqual(rc, 0) self.assertEqual(stdout.rstrip(), b"") - self.assertIn(b"Exception ignored in:", stderr) + self.assertIn(b"Exception ignored while calling deallocator", stderr) self.assertIn(b"C.__del__", stderr) def test__struct_reference_cycle_cleaned_up(self): diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index cf0f1d671b507a..d85dbfe52d2fe1 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -105,8 +105,10 @@ fileio_dealloc_warn(PyObject *op, PyObject *source) PyObject *exc = PyErr_GetRaisedException(); if (PyErr_ResourceWarning(source, 1, "unclosed file %R", source)) { /* Spurious errors can appear at shutdown */ - if (PyErr_ExceptionMatches(PyExc_Warning)) - PyErr_WriteUnraisable((PyObject *) self); + if (PyErr_ExceptionMatches(PyExc_Warning)) { + PyErr_FormatUnraisable("Exception ignored " + "while closing file %R", self); + } } PyErr_SetRaisedException(exc); } diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 419e5516b5c11e..537be1e7ba8888 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -314,7 +314,8 @@ iobase_finalize(PyObject *self) PyErr_Clear(); res = PyObject_CallMethodNoArgs((PyObject *)self, &_Py_ID(close)); if (res == NULL) { - PyErr_WriteUnraisable(self); + PyErr_FormatUnraisable("Exception ignored " + "while closing file %R", self); } else { Py_DECREF(res); diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index b178eb42ac8e6a..4b6d2dd1c5fc7b 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -5359,7 +5359,8 @@ sock_finalize(PyObject *self) if (PyErr_ResourceWarning((PyObject *)s, 1, "unclosed %R", s)) { /* Spurious errors can appear at shutdown */ if (PyErr_ExceptionMatches(PyExc_Warning)) { - PyErr_WriteUnraisable((PyObject *)s); + PyErr_FormatUnraisable("Exception ignored while " + "finalizing socket %R", s); } } diff --git a/Objects/genobject.c b/Objects/genobject.c index 73bbf86588c457..79aed8571c35e7 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -97,8 +97,10 @@ _PyGen_Finalize(PyObject *self) PyObject *res = PyObject_CallOneArg(finalizer, self); if (res == NULL) { - PyErr_WriteUnraisable(self); - } else { + PyErr_FormatUnraisable("Exception ignored while " + "finalizing generator %R", self); + } + else { Py_DECREF(res); } /* Restore the saved exception. */ @@ -122,7 +124,8 @@ _PyGen_Finalize(PyObject *self) PyObject *res = gen_close((PyObject*)gen, NULL); if (res == NULL) { if (PyErr_Occurred()) { - PyErr_WriteUnraisable(self); + PyErr_FormatUnraisable("Exception ignored while " + "closing generator %R", self); } } else { @@ -338,7 +341,8 @@ gen_close_iter(PyObject *yf) else { PyObject *meth; if (PyObject_GetOptionalAttr(yf, &_Py_ID(close), &meth) < 0) { - PyErr_WriteUnraisable(yf); + PyErr_FormatUnraisable("Exception ignored while " + "closing generator %R", yf); } if (meth) { retval = _PyObject_CallNoArgs(meth); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 93920341a179e8..f3238da8a642e4 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -10288,10 +10288,13 @@ slot_tp_finalize(PyObject *self) del = lookup_maybe_method(self, &_Py_ID(__del__), &unbound); if (del != NULL) { res = call_unbound_noarg(unbound, del, self); - if (res == NULL) - PyErr_WriteUnraisable(del); - else + if (res == NULL) { + PyErr_FormatUnraisable("Exception ignored while " + "calling deallocator %R", del); + } + else { Py_DECREF(res); + } Py_DECREF(del); } diff --git a/Python/_warnings.c b/Python/_warnings.c index 283f203c72c9bf..bb195da9512caf 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1445,7 +1445,8 @@ _PyErr_WarnUnawaitedAgenMethod(PyAsyncGenObject *agen, PyObject *method) "coroutine method %R of %R was never awaited", method, agen->ag_qualname) < 0) { - PyErr_WriteUnraisable((PyObject *)agen); + PyErr_FormatUnraisable("Exception ignored while " + "finalizing async generator %R", agen); } PyErr_SetRaisedException(exc); } @@ -1487,14 +1488,17 @@ _PyErr_WarnUnawaitedCoroutine(PyObject *coro) } if (PyErr_Occurred()) { - PyErr_WriteUnraisable(coro); + PyErr_FormatUnraisable("Exception ignored while " + "finalizing coroutine %R", coro); } + if (!warned) { if (_PyErr_WarnFormat(coro, PyExc_RuntimeWarning, 1, "coroutine '%S' was never awaited", ((PyCoroObject *)coro)->cr_qualname) < 0) { - PyErr_WriteUnraisable(coro); + PyErr_FormatUnraisable("Exception ignored while " + "finalizing coroutine %R", coro); } } } From 35bbe0ec516819129111a60a97f38cf2dcd30fff Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 4 Feb 2025 15:53:55 +0100 Subject: [PATCH 2/5] Revert changes done in separated PR --- Modules/_io/fileio.c | 6 ++---- Modules/_io/iobase.c | 3 +-- Modules/socketmodule.c | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index d85dbfe52d2fe1..cf0f1d671b507a 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -105,10 +105,8 @@ fileio_dealloc_warn(PyObject *op, PyObject *source) PyObject *exc = PyErr_GetRaisedException(); if (PyErr_ResourceWarning(source, 1, "unclosed file %R", source)) { /* Spurious errors can appear at shutdown */ - if (PyErr_ExceptionMatches(PyExc_Warning)) { - PyErr_FormatUnraisable("Exception ignored " - "while closing file %R", self); - } + if (PyErr_ExceptionMatches(PyExc_Warning)) + PyErr_WriteUnraisable((PyObject *) self); } PyErr_SetRaisedException(exc); } diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 537be1e7ba8888..419e5516b5c11e 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -314,8 +314,7 @@ iobase_finalize(PyObject *self) PyErr_Clear(); res = PyObject_CallMethodNoArgs((PyObject *)self, &_Py_ID(close)); if (res == NULL) { - PyErr_FormatUnraisable("Exception ignored " - "while closing file %R", self); + PyErr_WriteUnraisable(self); } else { Py_DECREF(res); diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 4b6d2dd1c5fc7b..b178eb42ac8e6a 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -5359,8 +5359,7 @@ sock_finalize(PyObject *self) if (PyErr_ResourceWarning((PyObject *)s, 1, "unclosed %R", s)) { /* Spurious errors can appear at shutdown */ if (PyErr_ExceptionMatches(PyExc_Warning)) { - PyErr_FormatUnraisable("Exception ignored while " - "finalizing socket %R", s); + PyErr_WriteUnraisable((PyObject *)s); } } From ff11ab30a379ab2f8f56412985e76f3d40466fee Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 4 Feb 2025 16:38:15 +0100 Subject: [PATCH 3/5] Check err_msg in tests --- Lib/test/test_coroutines.py | 7 +++++++ Lib/test/test_exceptions.py | 4 ++++ Lib/test/test_generators.py | 3 +++ 3 files changed, 14 insertions(+) diff --git a/Lib/test/test_coroutines.py b/Lib/test/test_coroutines.py index 50e3f60dde0bf4..5566c9803d43ed 100644 --- a/Lib/test/test_coroutines.py +++ b/Lib/test/test_coroutines.py @@ -2136,6 +2136,9 @@ async def func(): pass coro = None support.gc_collect() + self.assertEqual(cm.unraisable.err_msg, + f"Exception ignored while finalizing " + f"coroutine {coro_repr}") self.assertIn("was never awaited", str(cm.unraisable.exc_value)) def test_for_assign_raising_stop_async_iteration(self): @@ -2410,9 +2413,13 @@ async def corofn(): coro_repr = repr(coro) # clear reference to the coroutine without awaiting for it + coro_repr = repr(coro) del coro support.gc_collect() + self.assertEqual(cm.unraisable.err_msg, + f"Exception ignored while finalizing " + f"coroutine {coro_repr}") self.assertEqual(cm.unraisable.exc_type, ZeroDivisionError) del warnings._warn_unawaited_coroutine diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index ad6ff6714d2c13..3838eb5b27c9e6 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1678,9 +1678,13 @@ def __del__(self): obj = BrokenDel() with support.catch_unraisable_exception() as cm: + obj_repr = repr(type(obj).__del__) del obj gc_collect() # For PyPy or other GCs. + self.assertEqual(cm.unraisable.err_msg, + f"Exception ignored while calling " + f"deallocator {obj_repr}") self.assertIsNotNone(cm.unraisable.exc_traceback) def test_unhandled(self): diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 46b1988b70b51e..c7fa232c6cef22 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -2664,14 +2664,17 @@ def printsolution(self, x): >>> with support.catch_unraisable_exception() as cm: ... g = f() ... next(g) +... gen_repr = repr(g) ... del g ... +... cm.unraisable.err_msg == f'Exception ignored while closing generator {gen_repr}' ... cm.unraisable.exc_type == RuntimeError ... "generator ignored GeneratorExit" in str(cm.unraisable.exc_value) ... cm.unraisable.exc_traceback is not None True True True +True And errors thrown during closing should propagate: From 0456a0ea685dbca6e0d02a51f5bb5b4c7b01f0fe Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 4 Feb 2025 16:39:38 +0100 Subject: [PATCH 4/5] Truncate to 80 columns --- Lib/test/test_generators.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index c7fa232c6cef22..4f2ec03a3d3d08 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -2667,7 +2667,8 @@ def printsolution(self, x): ... gen_repr = repr(g) ... del g ... -... cm.unraisable.err_msg == f'Exception ignored while closing generator {gen_repr}' +... cm.unraisable.err_msg == (f'Exception ignored while closing ' +... f'generator {gen_repr}') ... cm.unraisable.exc_type == RuntimeError ... "generator ignored GeneratorExit" in str(cm.unraisable.exc_value) ... cm.unraisable.exc_traceback is not None From b37c9da65d66bacdb95a57e7259983c4afc1c108 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 5 Feb 2025 11:08:44 +0100 Subject: [PATCH 5/5] Add one more test --- Lib/test/test_generators.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 4f2ec03a3d3d08..bf4b88cd9c4450 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -2780,15 +2780,19 @@ def printsolution(self, x): ... invoke("del failed") ... >>> with support.catch_unraisable_exception() as cm: -... l = Leaker() -... del l +... leaker = Leaker() +... del_repr = repr(type(leaker).__del__) +... del leaker ... +... cm.unraisable.err_msg == (f'Exception ignored while ' +... f'calling deallocator {del_repr}') ... cm.unraisable.exc_type == RuntimeError ... str(cm.unraisable.exc_value) == "del failed" ... cm.unraisable.exc_traceback is not None True True True +True These refleak tests should perhaps be in a testfile of their own,