From 71c52e3048dd07567f0c690eab4e5d57be66f534 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 27 May 2019 08:57:14 +0200 Subject: bpo-36829: Add _PyErr_WriteUnraisableMsg() (GH-13488) * sys.unraisablehook: add 'err_msg' field to UnraisableHookArgs. * Use _PyErr_WriteUnraisableMsg() in _ctypes _DictRemover_call() and gc delete_garbage(). --- Doc/library/sys.rst | 5 +++ Include/cpython/pyerrors.h | 3 ++ Lib/test/test_sys.py | 44 +++++++++++++++--------- Modules/_ctypes/_ctypes.c | 6 ++-- Modules/_testcapimodule.c | 17 +++++++-- Modules/gcmodule.c | 5 ++- Python/clinic/sysmodule.c.h | 9 ++--- Python/errors.c | 84 ++++++++++++++++++++++++++++++++++++++------- Python/sysmodule.c | 9 ++--- 9 files changed, 135 insertions(+), 47 deletions(-) diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 0294f74..51a208e 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -1566,11 +1566,16 @@ always available. * *exc_type*: Exception type. * *exc_value*: Exception value, can be ``None``. * *exc_traceback*: Exception traceback, can be ``None``. + * *err_msg*: Error message, can be ``None``. * *object*: Object causing the exception, can be ``None``. :func:`sys.unraisablehook` can be overridden to control how unraisable exceptions are handled. + The default hook formats *err_msg* and *object* as: + ``f'{err_msg}: {object!r}'``; use "Exception ignored in" error message + if *err_msg* is ``None``. + See also :func:`excepthook` which handles uncaught exceptions. .. versionadded:: 3.8 diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h index de6548d..6b0cced 100644 --- a/Include/cpython/pyerrors.h +++ b/Include/cpython/pyerrors.h @@ -171,6 +171,9 @@ PyAPI_FUNC(PyObject *) _PyUnicodeTranslateError_Create( const char *reason /* UTF-8 encoded string */ ); +PyAPI_FUNC(void) _PyErr_WriteUnraisableMsg( + const char *err_msg, + PyObject *obj); #ifdef __cplusplus } diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 1e5f168..dfe63b1 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -878,31 +878,38 @@ class SysModuleTest(unittest.TestCase): @test.support.cpython_only class UnraisableHookTest(unittest.TestCase): - def write_unraisable_exc(self, exc, obj): + def write_unraisable_exc(self, exc, err_msg, obj): import _testcapi import types + err_msg2 = f"Exception ignored {err_msg}" try: - _testcapi.write_unraisable_exc(exc, obj) + _testcapi.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): - 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), obj) - - err = stderr.getvalue() - 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) + 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) def test_original_unraisablehook_err(self): # bpo-22836: PyErr_WriteUnraisable() should give sensible reports @@ -962,8 +969,9 @@ class UnraisableHookTest(unittest.TestCase): obj = object() try: with test.support.swap_attr(sys, 'unraisablehook', hook_func): - expected = self.write_unraisable_exc(ValueError(42), obj) - for attr in "exc_type exc_value exc_traceback object".split(): + 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)) @@ -978,10 +986,12 @@ class UnraisableHookTest(unittest.TestCase): with test.support.captured_output("stderr") as stderr: with test.support.swap_attr(sys, 'unraisablehook', hook_func): - self.write_unraisable_exc(ValueError(42), None) + self.write_unraisable_exc(ValueError(42), + "custom hook fail", None) err = stderr.getvalue() - self.assertIn(f'Exception ignored in: {hook_func!r}\n', + self.assertIn(f'Exception ignored in sys.unraisablehook: ' + f'{hook_func!r}\n', err) self.assertIn('Traceback (most recent call last):\n', err) self.assertIn('Exception: hook_func failed\n', err) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index f4eb536..21b08f8 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -150,9 +150,9 @@ _DictRemover_call(PyObject *myself, PyObject *args, PyObject *kw) { DictRemoverObject *self = (DictRemoverObject *)myself; if (self->key && self->dict) { - if (-1 == PyDict_DelItem(self->dict, self->key)) - /* XXX Error context */ - PyErr_WriteUnraisable(Py_None); + if (-1 == PyDict_DelItem(self->dict, self->key)) { + _PyErr_WriteUnraisableMsg("on calling _ctypes.DictRemover", NULL); + } Py_CLEAR(self->key); Py_CLEAR(self->dict); } diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 7945f49..51e5d80 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4985,13 +4985,24 @@ negative_refcount(PyObject *self, PyObject *Py_UNUSED(args)) static PyObject* test_write_unraisable_exc(PyObject *self, PyObject *args) { - PyObject *exc, *obj; - if (!PyArg_ParseTuple(args, "OO", &exc, &obj)) { + PyObject *exc, *err_msg, *obj; + if (!PyArg_ParseTuple(args, "OOO", &exc, &err_msg, &obj)) { return NULL; } + 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_WriteUnraisable(obj); + _PyErr_WriteUnraisableMsg(err_msg_utf8, obj); Py_RETURN_NONE; } diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index be9b73a..3b15c7b 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -929,9 +929,8 @@ delete_garbage(struct _gc_runtime_state *state, Py_INCREF(op); (void) clear(op); if (PyErr_Occurred()) { - PySys_WriteStderr("Exception ignored in tp_clear of " - "%.50s\n", Py_TYPE(op)->tp_name); - PyErr_WriteUnraisable(NULL); + _PyErr_WriteUnraisableMsg("in tp_clear of", + (PyObject*)Py_TYPE(op)); } Py_DECREF(op); } diff --git a/Python/clinic/sysmodule.c.h b/Python/clinic/sysmodule.c.h index 2a4ec72..5df8af1 100644 --- a/Python/clinic/sysmodule.c.h +++ b/Python/clinic/sysmodule.c.h @@ -106,9 +106,10 @@ PyDoc_STRVAR(sys_unraisablehook__doc__, "The unraisable argument has the following attributes:\n" "\n" "* exc_type: Exception type.\n" -"* exc_value: Exception value.\n" -"* exc_tb: Exception traceback, can be None.\n" -"* obj: Object causing the exception, can be None."); +"* exc_value: Exception value, can be None.\n" +"* exc_traceback: Exception traceback, can be None.\n" +"* err_msg: Error message, can be None.\n" +"* object: Object causing the exception, can be None."); #define SYS_UNRAISABLEHOOK_METHODDEF \ {"unraisablehook", (PyCFunction)sys_unraisablehook, METH_O, sys_unraisablehook__doc__}, @@ -1108,4 +1109,4 @@ sys_getandroidapilevel(PyObject *module, PyObject *Py_UNUSED(ignored)) #ifndef SYS_GETANDROIDAPILEVEL_METHODDEF #define SYS_GETANDROIDAPILEVEL_METHODDEF #endif /* !defined(SYS_GETANDROIDAPILEVEL_METHODDEF) */ -/*[clinic end generated code: output=3c32bc91ec659509 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=03da2eb03135d9f2 input=a9049054013a1b77]*/ diff --git a/Python/errors.c b/Python/errors.c index e721f19..831f111 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1077,6 +1077,7 @@ static PyStructSequence_Field UnraisableHookArgs_fields[] = { {"exc_type", "Exception type"}, {"exc_value", "Exception value"}, {"exc_traceback", "Exception traceback"}, + {"err_msg", "Error message"}, {"object", "Object causing the exception"}, {0} }; @@ -1085,7 +1086,7 @@ static PyStructSequence_Desc UnraisableHookArgs_desc = { .name = "UnraisableHookArgs", .doc = UnraisableHookArgs__doc__, .fields = UnraisableHookArgs_fields, - .n_in_sequence = 4 + .n_in_sequence = 5 }; @@ -1104,7 +1105,8 @@ _PyErr_Init(void) static PyObject * make_unraisable_hook_args(PyThreadState *tstate, PyObject *exc_type, - PyObject *exc_value, PyObject *exc_tb, PyObject *obj) + PyObject *exc_value, PyObject *exc_tb, + PyObject *err_msg, PyObject *obj) { PyObject *args = PyStructSequence_New(&UnraisableHookArgsType); if (args == NULL) { @@ -1125,6 +1127,7 @@ make_unraisable_hook_args(PyThreadState *tstate, PyObject *exc_type, ADD_ITEM(exc_type); ADD_ITEM(exc_value); ADD_ITEM(exc_tb); + ADD_ITEM(err_msg); ADD_ITEM(obj); #undef ADD_ITEM @@ -1145,11 +1148,21 @@ make_unraisable_hook_args(PyThreadState *tstate, PyObject *exc_type, static int write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type, PyObject *exc_value, PyObject *exc_tb, - PyObject *obj, PyObject *file) + PyObject *err_msg, PyObject *obj, PyObject *file) { if (obj != NULL && obj != Py_None) { - if (PyFile_WriteString("Exception ignored in: ", file) < 0) { - return -1; + if (err_msg != NULL && err_msg != Py_None) { + if (PyFile_WriteObject(err_msg, file, Py_PRINT_RAW) < 0) { + return -1; + } + if (PyFile_WriteString(": ", file) < 0) { + return -1; + } + } + else { + if (PyFile_WriteString("Exception ignored in: ", file) < 0) { + return -1; + } } if (PyFile_WriteObject(obj, file, 0) < 0) { @@ -1162,6 +1175,14 @@ write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type, return -1; } } + else if (err_msg != NULL && err_msg != Py_None) { + if (PyFile_WriteObject(err_msg, file, Py_PRINT_RAW) < 0) { + return -1; + } + if (PyFile_WriteString(":\n", file) < 0) { + return -1; + } + } if (exc_tb != NULL && exc_tb != Py_None) { if (PyTraceBack_Print(exc_tb, file) < 0) { @@ -1178,8 +1199,9 @@ write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type, const char *className = PyExceptionClass_Name(exc_type); if (className != NULL) { const char *dot = strrchr(className, '.'); - if (dot != NULL) + if (dot != NULL) { className = dot+1; + } } _Py_IDENTIFIER(__module__); @@ -1238,7 +1260,8 @@ write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type, static int write_unraisable_exc(PyThreadState *tstate, PyObject *exc_type, - PyObject *exc_value, PyObject *exc_tb, PyObject *obj) + PyObject *exc_value, PyObject *exc_tb, PyObject *err_msg, + PyObject *obj) { PyObject *file = _PySys_GetObjectId(&PyId_stderr); if (file == NULL || file == Py_None) { @@ -1249,7 +1272,7 @@ write_unraisable_exc(PyThreadState *tstate, PyObject *exc_type, while we use it */ Py_INCREF(file); int res = write_unraisable_exc_file(tstate, exc_type, exc_value, exc_tb, - obj, file); + err_msg, obj, file); Py_DECREF(file); return res; @@ -1272,9 +1295,10 @@ _PyErr_WriteUnraisableDefaultHook(PyObject *args) PyObject *exc_type = PyStructSequence_GET_ITEM(args, 0); PyObject *exc_value = PyStructSequence_GET_ITEM(args, 1); PyObject *exc_tb = PyStructSequence_GET_ITEM(args, 2); - PyObject *obj = PyStructSequence_GET_ITEM(args, 3); + PyObject *err_msg = PyStructSequence_GET_ITEM(args, 3); + PyObject *obj = PyStructSequence_GET_ITEM(args, 4); - if (write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, obj) < 0) { + if (write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, err_msg, obj) < 0) { return NULL; } Py_RETURN_NONE; @@ -1287,13 +1311,18 @@ _PyErr_WriteUnraisableDefaultHook(PyObject *args) for Python to handle it. For example, when a destructor raises an exception or during garbage collection (gc.collect()). + If err_msg_str is non-NULL, the error message is formatted as: + "Exception ignored %s" % err_msg_str. Otherwise, use "Exception ignored in" + error message. + An exception must be set when calling this function. */ void -PyErr_WriteUnraisable(PyObject *obj) +_PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj) { PyThreadState *tstate = _PyThreadState_GET(); assert(tstate != NULL); + PyObject *err_msg = NULL; PyObject *exc_type, *exc_value, *exc_tb; _PyErr_Fetch(tstate, &exc_type, &exc_value, &exc_tb); @@ -1322,13 +1351,20 @@ PyErr_WriteUnraisable(PyObject *obj) } } + if (err_msg_str != NULL) { + err_msg = PyUnicode_FromFormat("Exception ignored %s", err_msg_str); + if (err_msg == NULL) { + PyErr_Clear(); + } + } + _Py_IDENTIFIER(unraisablehook); PyObject *hook = _PySys_GetObjectId(&PyId_unraisablehook); if (hook != NULL && hook != Py_None) { PyObject *hook_args; hook_args = make_unraisable_hook_args(tstate, exc_type, exc_value, - exc_tb, obj); + exc_tb, err_msg, obj); if (hook_args != NULL) { PyObject *args[1] = {hook_args}; PyObject *res = _PyObject_FastCall(hook, args, 1); @@ -1337,6 +1373,18 @@ PyErr_WriteUnraisable(PyObject *obj) Py_DECREF(res); goto done; } + + err_msg_str = "Exception ignored in sys.unraisablehook"; + } + else { + err_msg_str = ("Exception ignored on building " + "sys.unraisablehook arguments"); + } + + Py_XDECREF(err_msg); + err_msg = PyUnicode_FromString(err_msg_str); + if (err_msg == NULL) { + PyErr_Clear(); } /* sys.unraisablehook failed: log its error using default hook */ @@ -1350,15 +1398,25 @@ PyErr_WriteUnraisable(PyObject *obj) default_hook: /* Call the default unraisable hook (ignore failure) */ - (void)write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, obj); + (void)write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, + err_msg, obj); done: Py_XDECREF(exc_type); Py_XDECREF(exc_value); Py_XDECREF(exc_tb); + Py_XDECREF(err_msg); _PyErr_Clear(tstate); /* Just in case */ } + +void +PyErr_WriteUnraisable(PyObject *obj) +{ + _PyErr_WriteUnraisableMsg(NULL, obj); +} + + extern PyObject *PyModule_GetWarningsModule(void); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 5ebeacf..08a1a29 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -689,14 +689,15 @@ Handle an unraisable exception. The unraisable argument has the following attributes: * exc_type: Exception type. -* exc_value: Exception value. -* exc_tb: Exception traceback, can be None. -* obj: Object causing the exception, can be None. +* exc_value: Exception value, can be None. +* exc_traceback: Exception traceback, can be None. +* err_msg: Error message, can be None. +* object: Object causing the exception, can be None. [clinic start generated code]*/ static PyObject * sys_unraisablehook(PyObject *module, PyObject *unraisable) -/*[clinic end generated code: output=bb92838b32abaa14 input=fdbdb47fdd0bee06]*/ +/*[clinic end generated code: output=bb92838b32abaa14 input=ec3af148294af8d3]*/ { return _PyErr_WriteUnraisableDefaultHook(unraisable); } -- cgit v0.12