Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-108082: Remove _PyErr_WriteUnraisableMsg() #111643

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions Include/internal/pycore_pyerrors.h
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading