From 26c0e5e03a8603eccfd98045bc69fde2e24682e3 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 3 Nov 2023 09:45:53 +0200 Subject: gh-108082: Remove _PyErr_WriteUnraisableMsg() (GH-111643) Replace the remaining calls with PyErr_FormatUnraisable(). --- Include/internal/pycore_pyerrors.h | 5 -- Lib/test/_test_atexit.py | 8 ++- Lib/test/audit-tests.py | 5 +- Lib/test/test_ctypes/test_callbacks.py | 6 +- Lib/test/test_ctypes/test_random_things.py | 6 +- Lib/test/test_sys.py | 97 +++++++++++++++++------------- Lib/test/test_thread.py | 4 +- Modules/_ctypes/callbacks.c | 20 +++--- Modules/_testinternalcapi.c | 32 ---------- Modules/_threadmodule.c | 4 +- Modules/atexitmodule.c | 4 +- Modules/clinic/_testinternalcapi.c.h | 34 +---------- Python/errors.c | 11 ---- 13 files changed, 87 insertions(+), 149 deletions(-) diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index 67ef71c..a953d2b 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -194,11 +194,6 @@ Py_DEPRECATED(3.12) extern void _PyErr_ChainExceptions(PyObject *, PyObject *, P // Export for '_zoneinfo' shared extension PyAPI_FUNC(void) _PyErr_ChainExceptions1(PyObject *); -// Export for '_lsprof' shared extension -PyAPI_FUNC(void) _PyErr_WriteUnraisableMsg( - const char *err_msg, - PyObject *obj); - #ifdef __cplusplus } #endif diff --git a/Lib/test/_test_atexit.py b/Lib/test/_test_atexit.py index 55d2808..f618c1f 100644 --- a/Lib/test/_test_atexit.py +++ b/Lib/test/_test_atexit.py @@ -19,7 +19,9 @@ class GeneralTest(unittest.TestCase): atexit.register(func, *args) atexit._run_exitfuncs() - self.assertEqual(cm.unraisable.object, func) + self.assertIsNone(cm.unraisable.object) + self.assertEqual(cm.unraisable.err_msg, + f'Exception ignored in atexit callback {func!r}') self.assertEqual(cm.unraisable.exc_type, exc_type) self.assertEqual(type(cm.unraisable.exc_value), exc_type) @@ -125,7 +127,9 @@ class GeneralTest(unittest.TestCase): try: with support.catch_unraisable_exception() as cm: atexit._run_exitfuncs() - self.assertEqual(cm.unraisable.object, func) + self.assertIsNone(cm.unraisable.object) + self.assertEqual(cm.unraisable.err_msg, + f'Exception ignored in atexit callback {func!r}') self.assertEqual(cm.unraisable.exc_type, ZeroDivisionError) self.assertEqual(type(cm.unraisable.exc_value), ZeroDivisionError) finally: diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py index f0cedde..89f407d 100644 --- a/Lib/test/audit-tests.py +++ b/Lib/test/audit-tests.py @@ -289,7 +289,7 @@ def test_excepthook(): def test_unraisablehook(): - from _testinternalcapi import write_unraisable_exc + from _testcapi import err_formatunraisable def unraisablehook(hookargs): pass @@ -302,7 +302,8 @@ def test_unraisablehook(): sys.addaudithook(hook) sys.unraisablehook = unraisablehook - write_unraisable_exc(RuntimeError("nonfatal-error"), "for audit hook test", None) + err_formatunraisable(RuntimeError("nonfatal-error"), + "Exception ignored for audit hook test") def test_winreg(): diff --git a/Lib/test/test_ctypes/test_callbacks.py b/Lib/test/test_ctypes/test_callbacks.py index 6fe3e11..19f4158 100644 --- a/Lib/test/test_ctypes/test_callbacks.py +++ b/Lib/test/test_ctypes/test_callbacks.py @@ -322,9 +322,9 @@ class SampleCallbacksTestCase(unittest.TestCase): self.assertIsInstance(cm.unraisable.exc_value, TypeError) self.assertEqual(cm.unraisable.err_msg, - "Exception ignored on converting result " - "of ctypes callback function") - self.assertIs(cm.unraisable.object, func) + f"Exception ignored on converting result " + f"of ctypes callback function {func!r}") + self.assertIsNone(cm.unraisable.object) if __name__ == '__main__': diff --git a/Lib/test/test_ctypes/test_random_things.py b/Lib/test/test_ctypes/test_random_things.py index 65eb53f..630f6ed 100644 --- a/Lib/test/test_ctypes/test_random_things.py +++ b/Lib/test/test_ctypes/test_random_things.py @@ -51,9 +51,9 @@ class CallbackTracbackTestCase(unittest.TestCase): if exc_msg is not None: self.assertEqual(str(cm.unraisable.exc_value), exc_msg) self.assertEqual(cm.unraisable.err_msg, - "Exception ignored on calling ctypes " - "callback function") - self.assertIs(cm.unraisable.object, callback_func) + f"Exception ignored on calling ctypes " + f"callback function {callback_func!r}") + self.assertIsNone(cm.unraisable.object) def test_ValueError(self): cb = CFUNCTYPE(c_int, c_int)(callback_func) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index b16e1e7..b111962 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1215,38 +1215,38 @@ class SysModuleTest(unittest.TestCase): @test.support.cpython_only class UnraisableHookTest(unittest.TestCase): - def write_unraisable_exc(self, exc, err_msg, obj): - import _testinternalcapi - import types - err_msg2 = f"Exception ignored {err_msg}" - try: - _testinternalcapi.write_unraisable_exc(exc, err_msg, obj) - return types.SimpleNamespace(exc_type=type(exc), - exc_value=exc, - exc_traceback=exc.__traceback__, - err_msg=err_msg2, - object=obj) - finally: - # Explicitly break any reference cycle - exc = None - def test_original_unraisablehook(self): - for err_msg in (None, "original hook"): - with self.subTest(err_msg=err_msg): - obj = "an object" - - with test.support.captured_output("stderr") as stderr: - with test.support.swap_attr(sys, 'unraisablehook', - sys.__unraisablehook__): - self.write_unraisable_exc(ValueError(42), err_msg, obj) - - err = stderr.getvalue() - if err_msg is not None: - self.assertIn(f'Exception ignored {err_msg}: {obj!r}\n', err) - else: - self.assertIn(f'Exception ignored in: {obj!r}\n', err) - self.assertIn('Traceback (most recent call last):\n', err) - self.assertIn('ValueError: 42\n', err) + _testcapi = import_helper.import_module('_testcapi') + from _testcapi import err_writeunraisable, err_formatunraisable + obj = hex + + with support.swap_attr(sys, 'unraisablehook', + sys.__unraisablehook__): + with support.captured_stderr() as stderr: + err_writeunraisable(ValueError(42), obj) + lines = stderr.getvalue().splitlines() + self.assertEqual(lines[0], f'Exception ignored in: {obj!r}') + self.assertEqual(lines[1], 'Traceback (most recent call last):') + self.assertEqual(lines[-1], 'ValueError: 42') + + with support.captured_stderr() as stderr: + err_writeunraisable(ValueError(42), None) + lines = stderr.getvalue().splitlines() + self.assertEqual(lines[0], 'Traceback (most recent call last):') + self.assertEqual(lines[-1], 'ValueError: 42') + + with support.captured_stderr() as stderr: + err_formatunraisable(ValueError(42), 'Error in %R', obj) + lines = stderr.getvalue().splitlines() + self.assertEqual(lines[0], f'Error in {obj!r}:') + self.assertEqual(lines[1], 'Traceback (most recent call last):') + self.assertEqual(lines[-1], 'ValueError: 42') + + with support.captured_stderr() as stderr: + err_formatunraisable(ValueError(42), None) + lines = stderr.getvalue().splitlines() + self.assertEqual(lines[0], 'Traceback (most recent call last):') + self.assertEqual(lines[-1], 'ValueError: 42') def test_original_unraisablehook_err(self): # bpo-22836: PyErr_WriteUnraisable() should give sensible reports @@ -1293,6 +1293,8 @@ class UnraisableHookTest(unittest.TestCase): # Check that the exception is printed with its qualified name # rather than just classname, and the module names appears # unless it is one of the hard-coded exclusions. + _testcapi = import_helper.import_module('_testcapi') + from _testcapi import err_writeunraisable class A: class B: class X(Exception): @@ -1304,9 +1306,7 @@ class UnraisableHookTest(unittest.TestCase): with test.support.captured_stderr() as stderr, test.support.swap_attr( sys, 'unraisablehook', sys.__unraisablehook__ ): - expected = self.write_unraisable_exc( - A.B.X(), "msg", "obj" - ) + err_writeunraisable(A.B.X(), "obj") report = stderr.getvalue() self.assertIn(A.B.X.__qualname__, report) if moduleName in ['builtins', '__main__']: @@ -1322,34 +1322,45 @@ class UnraisableHookTest(unittest.TestCase): sys.unraisablehook(exc) def test_custom_unraisablehook(self): + _testcapi = import_helper.import_module('_testcapi') + from _testcapi import err_writeunraisable, err_formatunraisable hook_args = None def hook_func(args): nonlocal hook_args hook_args = args - obj = object() + obj = hex try: with test.support.swap_attr(sys, 'unraisablehook', hook_func): - expected = self.write_unraisable_exc(ValueError(42), - "custom hook", obj) - for attr in "exc_type exc_value exc_traceback err_msg object".split(): - self.assertEqual(getattr(hook_args, attr), - getattr(expected, attr), - (hook_args, expected)) + exc = ValueError(42) + err_writeunraisable(exc, obj) + self.assertIs(hook_args.exc_type, type(exc)) + self.assertIs(hook_args.exc_value, exc) + self.assertIs(hook_args.exc_traceback, exc.__traceback__) + self.assertIsNone(hook_args.err_msg) + self.assertEqual(hook_args.object, obj) + + err_formatunraisable(exc, "custom hook %R", obj) + self.assertIs(hook_args.exc_type, type(exc)) + self.assertIs(hook_args.exc_value, exc) + self.assertIs(hook_args.exc_traceback, exc.__traceback__) + self.assertEqual(hook_args.err_msg, f'custom hook {obj!r}') + self.assertIsNone(hook_args.object) finally: # expected and hook_args contain an exception: break reference cycle expected = None hook_args = None def test_custom_unraisablehook_fail(self): + _testcapi = import_helper.import_module('_testcapi') + from _testcapi import err_writeunraisable def hook_func(*args): raise Exception("hook_func failed") with test.support.captured_output("stderr") as stderr: with test.support.swap_attr(sys, 'unraisablehook', hook_func): - self.write_unraisable_exc(ValueError(42), - "custom hook fail", None) + err_writeunraisable(ValueError(42), "custom hook fail") err = stderr.getvalue() self.assertIn(f'Exception ignored in sys.unraisablehook: ' diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py index 8656fbd..831aaf5 100644 --- a/Lib/test/test_thread.py +++ b/Lib/test/test_thread.py @@ -155,9 +155,9 @@ class ThreadRunningTests(BasicThreadTest): started.acquire() self.assertEqual(str(cm.unraisable.exc_value), "task failed") - self.assertIs(cm.unraisable.object, task) + self.assertIsNone(cm.unraisable.object) self.assertEqual(cm.unraisable.err_msg, - "Exception ignored in thread started by") + f"Exception ignored in thread started by {task!r}") self.assertIsNotNone(cm.unraisable.exc_traceback) diff --git a/Modules/_ctypes/callbacks.c b/Modules/_ctypes/callbacks.c index 1bd8fec..154e9f4 100644 --- a/Modules/_ctypes/callbacks.c +++ b/Modules/_ctypes/callbacks.c @@ -9,7 +9,6 @@ #endif #include "pycore_call.h" // _PyObject_CallNoArgs() -#include "pycore_pyerrors.h" // _PyErr_WriteUnraisableMsg() #include "pycore_runtime.h" // _Py_ID() #include @@ -216,8 +215,9 @@ static void _CallPythonObject(void *mem, result = PyObject_Vectorcall(callable, args, nargs, NULL); if (result == NULL) { - _PyErr_WriteUnraisableMsg("on calling ctypes callback function", - callable); + PyErr_FormatUnraisable( + "Exception ignored on calling ctypes callback function %R", + callable); } #ifdef MS_WIN32 @@ -258,9 +258,10 @@ static void _CallPythonObject(void *mem, if (keep == NULL) { /* Could not convert callback result. */ - _PyErr_WriteUnraisableMsg("on converting result " - "of ctypes callback function", - callable); + PyErr_FormatUnraisable( + "Exception ignored on converting result " + "of ctypes callback function %R", + callable); } else if (setfunc != _ctypes_get_fielddesc("O")->setfunc) { if (keep == Py_None) { @@ -270,9 +271,10 @@ static void _CallPythonObject(void *mem, else if (PyErr_WarnEx(PyExc_RuntimeWarning, "memory leak in callback function.", 1) == -1) { - _PyErr_WriteUnraisableMsg("on converting result " - "of ctypes callback function", - callable); + PyErr_FormatUnraisable( + "Exception ignored on converting result " + "of ctypes callback function %R", + callable); } } } diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index a71e7e1..7dba29a 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1518,37 +1518,6 @@ restore_crossinterp_data(PyObject *self, PyObject *args) } -/*[clinic input] -_testinternalcapi.write_unraisable_exc - exception as exc: object - err_msg: object - obj: object - / -[clinic start generated code]*/ - -static PyObject * -_testinternalcapi_write_unraisable_exc_impl(PyObject *module, PyObject *exc, - PyObject *err_msg, PyObject *obj) -/*[clinic end generated code: output=a0f063cdd04aad83 input=274381b1a3fa5cd6]*/ -{ - - const char *err_msg_utf8; - if (err_msg != Py_None) { - err_msg_utf8 = PyUnicode_AsUTF8(err_msg); - if (err_msg_utf8 == NULL) { - return NULL; - } - } - else { - err_msg_utf8 = NULL; - } - - PyErr_SetObject((PyObject *)Py_TYPE(exc), exc); - _PyErr_WriteUnraisableMsg(err_msg_utf8, obj); - Py_RETURN_NONE; -} - - static PyObject * raiseTestError(const char* test_name, const char* msg) { @@ -1699,7 +1668,6 @@ static PyMethodDef module_functions[] = { {"perf_trampoline_set_persist_after_fork", perf_trampoline_set_persist_after_fork, METH_VARARGS}, {"get_crossinterp_data", get_crossinterp_data, METH_VARARGS}, {"restore_crossinterp_data", restore_crossinterp_data, METH_VARARGS}, - _TESTINTERNALCAPI_WRITE_UNRAISABLE_EXC_METHODDEF _TESTINTERNALCAPI_TEST_LONG_NUMBITS_METHODDEF {NULL, NULL} /* sentinel */ }; diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index a849a20..9eecebd 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -5,7 +5,6 @@ #include "Python.h" #include "pycore_interp.h" // _PyInterpreterState.threads.count #include "pycore_moduleobject.h" // _PyModule_GetState() -#include "pycore_pyerrors.h" // _PyErr_WriteUnraisableMsg() #include "pycore_pylifecycle.h" #include "pycore_pystate.h" // _PyThreadState_SetCurrent() #include "pycore_sysmodule.h" // _PySys_GetAttr() @@ -1071,7 +1070,8 @@ thread_run(void *boot_raw) /* SystemExit is ignored silently */ PyErr_Clear(); else { - _PyErr_WriteUnraisableMsg("in thread started by", boot->func); + PyErr_FormatUnraisable( + "Exception ignored in thread started by %R", boot->func); } } else { diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 57e2ea6..b6f1bcb 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -10,7 +10,6 @@ #include "pycore_atexit.h" // export _Py_AtExit() #include "pycore_initconfig.h" // _PyStatus_NO_MEMORY #include "pycore_interp.h" // PyInterpreterState.atexit -#include "pycore_pyerrors.h" // _PyErr_WriteUnraisableMsg() #include "pycore_pystate.h" // _PyInterpreterState_GET /* ===================================================================== */ @@ -137,7 +136,8 @@ atexit_callfuncs(struct atexit_state *state) PyObject* the_func = Py_NewRef(cb->func); PyObject *res = PyObject_Call(cb->func, cb->args, cb->kwargs); if (res == NULL) { - _PyErr_WriteUnraisableMsg("in atexit callback", the_func); + PyErr_FormatUnraisable( + "Exception ignored in atexit callback %R", the_func); } else { Py_DECREF(res); diff --git a/Modules/clinic/_testinternalcapi.c.h b/Modules/clinic/_testinternalcapi.c.h index 10374e0..cba2a94 100644 --- a/Modules/clinic/_testinternalcapi.c.h +++ b/Modules/clinic/_testinternalcapi.c.h @@ -266,38 +266,6 @@ exit: return return_value; } -PyDoc_STRVAR(_testinternalcapi_write_unraisable_exc__doc__, -"write_unraisable_exc($module, exception, err_msg, obj, /)\n" -"--\n" -"\n"); - -#define _TESTINTERNALCAPI_WRITE_UNRAISABLE_EXC_METHODDEF \ - {"write_unraisable_exc", _PyCFunction_CAST(_testinternalcapi_write_unraisable_exc), METH_FASTCALL, _testinternalcapi_write_unraisable_exc__doc__}, - -static PyObject * -_testinternalcapi_write_unraisable_exc_impl(PyObject *module, PyObject *exc, - PyObject *err_msg, PyObject *obj); - -static PyObject * -_testinternalcapi_write_unraisable_exc(PyObject *module, PyObject *const *args, Py_ssize_t nargs) -{ - PyObject *return_value = NULL; - PyObject *exc; - PyObject *err_msg; - PyObject *obj; - - if (!_PyArg_CheckPositional("write_unraisable_exc", nargs, 3, 3)) { - goto exit; - } - exc = args[0]; - err_msg = args[1]; - obj = args[2]; - return_value = _testinternalcapi_write_unraisable_exc_impl(module, exc, err_msg, obj); - -exit: - return return_value; -} - PyDoc_STRVAR(_testinternalcapi_test_long_numbits__doc__, "test_long_numbits($module, /)\n" "--\n" @@ -314,4 +282,4 @@ _testinternalcapi_test_long_numbits(PyObject *module, PyObject *Py_UNUSED(ignore { return _testinternalcapi_test_long_numbits_impl(module); } -/*[clinic end generated code: output=3425f97821fc7462 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=679bf53bbae20085 input=a9049054013a1b77]*/ diff --git a/Python/errors.c b/Python/errors.c index 30be7fa..c55ebfd 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1699,17 +1699,6 @@ format_unraisable(PyObject *obj, const char *format, ...) } void -_PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj) -{ - if (err_msg_str) { - format_unraisable(obj, "Exception ignored %s", err_msg_str); - } - else { - format_unraisable(obj, NULL); - } -} - -void PyErr_WriteUnraisable(PyObject *obj) { format_unraisable(obj, NULL); -- cgit v0.12