Skip to content

Commit

Permalink
pythongh-108082: Remove _PyErr_WriteUnraisableMsg() (pythonGH-111643)
Browse files Browse the repository at this point in the history
Replace the remaining calls with PyErr_FormatUnraisable().
  • Loading branch information
serhiy-storchaka authored and aisk committed Feb 11, 2024
1 parent 79d70fb commit 7611523
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 149 deletions.
5 changes: 0 additions & 5 deletions Include/internal/pycore_pyerrors.h
Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions Lib/test/_test_atexit.py
Expand Up @@ -19,7 +19,9 @@ def assert_raises_unraisable(self, exc_type, func, *args):
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)

Expand Down Expand Up @@ -125,7 +127,9 @@ def func():
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:
Expand Down
5 changes: 3 additions & 2 deletions Lib/test/audit-tests.py
Expand Up @@ -289,7 +289,7 @@ def hook(event, args):


def test_unraisablehook():
from _testinternalcapi import write_unraisable_exc
from _testcapi import err_formatunraisable

def unraisablehook(hookargs):
pass
Expand All @@ -302,7 +302,8 @@ def hook(event, args):

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():
Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_ctypes/test_callbacks.py
Expand Up @@ -322,9 +322,9 @@ def func():

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__':
Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_ctypes/test_random_things.py
Expand Up @@ -51,9 +51,9 @@ def expect_unraisable(self, exc_type, exc_msg=None):
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)
Expand Down
97 changes: 54 additions & 43 deletions Lib/test/test_sys.py
Expand Up @@ -1215,38 +1215,38 @@ def test_disable_gil_abi(self):

@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
Expand Down Expand Up @@ -1293,6 +1293,8 @@ def test_original_unraisablehook_exception_qualname(self):
# 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):
Expand All @@ -1304,9 +1306,7 @@ class X(Exception):
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__']:
Expand All @@ -1322,34 +1322,45 @@ def test_original_unraisablehook_wrong_type(self):
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: '
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_thread.py
Expand Up @@ -155,9 +155,9 @@ def task():
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)


Expand Down
20 changes: 11 additions & 9 deletions Modules/_ctypes/callbacks.c
Expand Up @@ -9,7 +9,6 @@
#endif

#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_pyerrors.h" // _PyErr_WriteUnraisableMsg()
#include "pycore_runtime.h" // _Py_ID()

#include <stdbool.h>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
}
Expand Down
32 changes: 0 additions & 32 deletions Modules/_testinternalcapi.c
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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 */
};
Expand Down
4 changes: 2 additions & 2 deletions Modules/_threadmodule.c
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions Modules/atexitmodule.c
Expand Up @@ -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

/* ===================================================================== */
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 7611523

Please sign in to comment.