From cd590a7cede156a4244e7cac61e4504e5344d842 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 28 May 2019 00:39:52 +0200 Subject: bpo-1230540: Add threading.excepthook() (GH-13515) Add a new threading.excepthook() function which handles uncaught Thread.run() exception. It can be overridden to control how uncaught exceptions are handled. threading.ExceptHookArgs is not documented on purpose: it should not be used directly. * threading.excepthook() and threading.ExceptHookArgs. * Add _PyErr_Display(): similar to PyErr_Display(), but accept a 'file' parameter. * Add _thread._excepthook(): C implementation of the exception hook calling _PyErr_Display(). * Add _thread._ExceptHookArgs: structseq type. * Add threading._invoke_excepthook_wrapper() which handles the gory details to ensure that everything remains alive during Python shutdown. * Add unit tests. --- Doc/library/sys.rst | 6 +- Doc/library/threading.rst | 30 ++++ Doc/whatsnew/3.8.rst | 9 ++ Include/internal/pycore_pylifecycle.h | 2 + Lib/test/test_threading.py | 92 ++++++++++++ Lib/threading.py | 155 +++++++++++++------- .../2019-05-23-01-48-39.bpo-1230540.oKTNEQ.rst | 3 + Modules/_threadmodule.c | 156 +++++++++++++++++++++ Python/pythonrun.c | 38 +++-- 9 files changed, 424 insertions(+), 67 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-05-23-01-48-39.bpo-1230540.oKTNEQ.rst diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 51a208e..74aa271 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -298,7 +298,11 @@ always available. before the program exits. The handling of such top-level exceptions can be customized by assigning another three-argument function to ``sys.excepthook``. - See also :func:`unraisablehook` which handles unraisable exceptions. + .. seealso:: + + The :func:`sys.unraisablehook` function handles unraisable exceptions + and the :func:`threading.excepthook` function handles exception raised + by :func:`threading.Thread.run`. .. data:: __breakpointhook__ diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 1df512f..ffe6d04 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -38,6 +38,32 @@ This module defines the following functions: returned. +.. function:: excepthook(args, /) + + Handle uncaught exception raised by :func:`Thread.run`. + + The *args* argument has the following attributes: + + * *exc_type*: Exception type. + * *exc_value*: Exception value, can be ``None``. + * *exc_traceback*: Exception traceback, can be ``None``. + * *thread*: Thread which raised the exception, can be ``None``. + + If *exc_type* is :exc:`SystemExit`, the exception is silently ignored. + Otherwise, the exception is printed out on :data:`sys.stderr`. + + If this function raises an exception, :func:`sys.excepthook` is called to + handle it. + + :func:`threading.excepthook` can be overridden to control how uncaught + exceptions raised by :func:`Thread.run` are handled. + + .. seealso:: + :func:`sys.excepthook` handles uncaught exceptions. + + .. versionadded:: 3.8 + + .. function:: get_ident() Return the 'thread identifier' of the current thread. This is a nonzero @@ -191,6 +217,10 @@ called is terminated. A thread has a name. The name can be passed to the constructor, and read or changed through the :attr:`~Thread.name` attribute. +If the :meth:`~Thread.run` method raises an exception, +:func:`threading.excepthook` is called to handle it. By default, +:func:`threading.excepthook` ignores silently :exc:`SystemExit`. + A thread can be flagged as a "daemon thread". The significance of this flag is that the entire Python program exits when only daemon threads are left. The initial value is inherited from the creating thread. The flag can be set diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index fd5e649..125cefe 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -623,6 +623,15 @@ in a standardized and extensible format, and offers several other benefits. (Contributed by C.A.M. Gerlach in :issue:`36268`.) +threading +--------- + +Add a new :func:`threading.excepthook` function which handles uncaught +:meth:`threading.Thread.run` exception. It can be overridden to control how +uncaught :meth:`threading.Thread.run` exceptions are handled. +(Contributed by Victor Stinner in :issue:`1230540`.) + + tokenize -------- diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index 69761ce..8a692ea 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -107,6 +107,8 @@ PyAPI_FUNC(int) _Py_HandleSystemExit(int *exitcode_p); PyAPI_FUNC(PyObject*) _PyErr_WriteUnraisableDefaultHook(PyObject *unraisable); PyAPI_FUNC(void) _PyErr_Print(PyThreadState *tstate); +PyAPI_FUNC(void) _PyErr_Display(PyObject *file, PyObject *exception, + PyObject *value, PyObject *tb); #ifdef __cplusplus } diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 3bfd6fa..8c8cc12 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1112,6 +1112,98 @@ class ThreadingExceptionTests(BaseTestCase): # explicitly break the reference cycle to not leak a dangling thread thread.exc = None + +class ThreadRunFail(threading.Thread): + def run(self): + raise ValueError("run failed") + + +class ExceptHookTests(BaseTestCase): + def test_excepthook(self): + with support.captured_output("stderr") as stderr: + thread = ThreadRunFail(name="excepthook thread") + thread.start() + thread.join() + + stderr = stderr.getvalue().strip() + self.assertIn(f'Exception in thread {thread.name}:\n', stderr) + self.assertIn('Traceback (most recent call last):\n', stderr) + self.assertIn(' raise ValueError("run failed")', stderr) + self.assertIn('ValueError: run failed', stderr) + + @support.cpython_only + def test_excepthook_thread_None(self): + # threading.excepthook called with thread=None: log the thread + # identifier in this case. + with support.captured_output("stderr") as stderr: + try: + raise ValueError("bug") + except Exception as exc: + args = threading.ExceptHookArgs([*sys.exc_info(), None]) + threading.excepthook(args) + + stderr = stderr.getvalue().strip() + self.assertIn(f'Exception in thread {threading.get_ident()}:\n', stderr) + self.assertIn('Traceback (most recent call last):\n', stderr) + self.assertIn(' raise ValueError("bug")', stderr) + self.assertIn('ValueError: bug', stderr) + + def test_system_exit(self): + class ThreadExit(threading.Thread): + def run(self): + sys.exit(1) + + # threading.excepthook() silently ignores SystemExit + with support.captured_output("stderr") as stderr: + thread = ThreadExit() + thread.start() + thread.join() + + self.assertEqual(stderr.getvalue(), '') + + def test_custom_excepthook(self): + args = None + + def hook(hook_args): + nonlocal args + args = hook_args + + try: + with support.swap_attr(threading, 'excepthook', hook): + thread = ThreadRunFail() + thread.start() + thread.join() + + self.assertEqual(args.exc_type, ValueError) + self.assertEqual(str(args.exc_value), 'run failed') + self.assertEqual(args.exc_traceback, args.exc_value.__traceback__) + self.assertIs(args.thread, thread) + finally: + # Break reference cycle + args = None + + def test_custom_excepthook_fail(self): + def threading_hook(args): + raise ValueError("threading_hook failed") + + err_str = None + + def sys_hook(exc_type, exc_value, exc_traceback): + nonlocal err_str + err_str = str(exc_value) + + with support.swap_attr(threading, 'excepthook', threading_hook), \ + support.swap_attr(sys, 'excepthook', sys_hook), \ + support.captured_output('stderr') as stderr: + thread = ThreadRunFail() + thread.start() + thread.join() + + self.assertEqual(stderr.getvalue(), + 'Exception in threading.excepthook:\n') + self.assertEqual(err_str, 'threading_hook failed') + + class TimerTests(BaseTestCase): def setUp(self): diff --git a/Lib/threading.py b/Lib/threading.py index 77a2bae..3d197ee 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -5,7 +5,6 @@ import sys as _sys import _thread from time import monotonic as _time -from traceback import format_exc as _format_exc from _weakrefset import WeakSet from itertools import islice as _islice, count as _count try: @@ -27,7 +26,8 @@ __all__ = ['get_ident', 'active_count', 'Condition', 'current_thread', 'enumerate', 'main_thread', 'TIMEOUT_MAX', 'Event', 'Lock', 'RLock', 'Semaphore', 'BoundedSemaphore', 'Thread', 'Barrier', 'BrokenBarrierError', 'Timer', 'ThreadError', - 'setprofile', 'settrace', 'local', 'stack_size'] + 'setprofile', 'settrace', 'local', 'stack_size', + 'excepthook', 'ExceptHookArgs'] # Rename some stuff so "from threading import *" is safe _start_new_thread = _thread.start_new_thread @@ -752,14 +752,6 @@ class Thread: """ _initialized = False - # Need to store a reference to sys.exc_info for printing - # out exceptions when a thread tries to use a global var. during interp. - # shutdown and thus raises an exception about trying to perform some - # operation on/with a NoneType - _exc_info = _sys.exc_info - # Keep sys.exc_clear too to clear the exception just before - # allowing .join() to return. - #XXX __exc_clear = _sys.exc_clear def __init__(self, group=None, target=None, name=None, args=(), kwargs=None, *, daemon=None): @@ -802,9 +794,9 @@ class Thread: self._started = Event() self._is_stopped = False self._initialized = True - # sys.stderr is not stored in the class like - # sys.exc_info since it can be changed between instances + # Copy of sys.stderr used by self._invoke_excepthook() self._stderr = _sys.stderr + self._invoke_excepthook = _make_invoke_excepthook() # For debugging and _after_fork() _dangling.add(self) @@ -929,47 +921,8 @@ class Thread: try: self.run() - except SystemExit: - pass except: - # If sys.stderr is no more (most likely from interpreter - # shutdown) use self._stderr. Otherwise still use sys (as in - # _sys) in case sys.stderr was redefined since the creation of - # self. - if _sys and _sys.stderr is not None: - print("Exception in thread %s:\n%s" % - (self.name, _format_exc()), file=_sys.stderr) - elif self._stderr is not None: - # Do the best job possible w/o a huge amt. of code to - # approximate a traceback (code ideas from - # Lib/traceback.py) - exc_type, exc_value, exc_tb = self._exc_info() - try: - print(( - "Exception in thread " + self.name + - " (most likely raised during interpreter shutdown):"), file=self._stderr) - print(( - "Traceback (most recent call last):"), file=self._stderr) - while exc_tb: - print(( - ' File "%s", line %s, in %s' % - (exc_tb.tb_frame.f_code.co_filename, - exc_tb.tb_lineno, - exc_tb.tb_frame.f_code.co_name)), file=self._stderr) - exc_tb = exc_tb.tb_next - print(("%s: %s" % (exc_type, exc_value)), file=self._stderr) - self._stderr.flush() - # Make sure that exc_tb gets deleted since it is a memory - # hog; deleting everything else is just for thoroughness - finally: - del exc_type, exc_value, exc_tb - finally: - # Prevent a race in - # test_threading.test_no_refcycle_through_target when - # the exception keeps the target alive past when we - # assert that it's dead. - #XXX self._exc_clear() - pass + self._invoke_excepthook(self) finally: with _active_limbo_lock: try: @@ -1163,6 +1116,104 @@ class Thread: def setName(self, name): self.name = name + +try: + from _thread import (_excepthook as excepthook, + _ExceptHookArgs as ExceptHookArgs) +except ImportError: + # Simple Python implementation if _thread._excepthook() is not available + from traceback import print_exception as _print_exception + from collections import namedtuple + + _ExceptHookArgs = namedtuple( + 'ExceptHookArgs', + 'exc_type exc_value exc_traceback thread') + + def ExceptHookArgs(args): + return _ExceptHookArgs(*args) + + def excepthook(args, /): + """ + Handle uncaught Thread.run() exception. + """ + if args.exc_type == SystemExit: + # silently ignore SystemExit + return + + if _sys is not None and _sys.stderr is not None: + stderr = _sys.stderr + elif args.thread is not None: + stderr = args.thread._stderr + if stderr is None: + # do nothing if sys.stderr is None and sys.stderr was None + # when the thread was created + return + else: + # do nothing if sys.stderr is None and args.thread is None + return + + if args.thread is not None: + name = args.thread.name + else: + name = get_ident() + print(f"Exception in thread {name}:", + file=stderr, flush=True) + _print_exception(args.exc_type, args.exc_value, args.exc_traceback, + file=stderr) + stderr.flush() + + +def _make_invoke_excepthook(): + # Create a local namespace to ensure that variables remain alive + # when _invoke_excepthook() is called, even if it is called late during + # Python shutdown. It is mostly needed for daemon threads. + + old_excepthook = excepthook + old_sys_excepthook = _sys.excepthook + if old_excepthook is None: + raise RuntimeError("threading.excepthook is None") + if old_sys_excepthook is None: + raise RuntimeError("sys.excepthook is None") + + sys_exc_info = _sys.exc_info + local_print = print + local_sys = _sys + + def invoke_excepthook(thread): + global excepthook + try: + hook = excepthook + if hook is None: + hook = old_excepthook + + args = ExceptHookArgs([*sys_exc_info(), thread]) + + hook(args) + except Exception as exc: + exc.__suppress_context__ = True + del exc + + if local_sys is not None and local_sys.stderr is not None: + stderr = local_sys.stderr + else: + stderr = thread._stderr + + local_print("Exception in threading.excepthook:", + file=stderr, flush=True) + + if local_sys is not None and local_sys.excepthook is not None: + sys_excepthook = local_sys.excepthook + else: + sys_excepthook = old_sys_excepthook + + sys_excepthook(*sys_exc_info()) + finally: + # Break reference cycle (exception stored in a variable) + args = None + + return invoke_excepthook + + # The timer class was contributed by Itamar Shtull-Trauring class Timer(Thread): diff --git a/Misc/NEWS.d/next/Library/2019-05-23-01-48-39.bpo-1230540.oKTNEQ.rst b/Misc/NEWS.d/next/Library/2019-05-23-01-48-39.bpo-1230540.oKTNEQ.rst new file mode 100644 index 0000000..250a642 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-05-23-01-48-39.bpo-1230540.oKTNEQ.rst @@ -0,0 +1,3 @@ +Add a new :func:`threading.excepthook` function which handles uncaught +:meth:`threading.Thread.run` exception. It can be overridden to control how +uncaught :meth:`threading.Thread.run` exceptions are handled. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index fee25ab..680e8ca 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -3,6 +3,7 @@ /* Interface to Sjoerd's portable C thread library */ #include "Python.h" +#include "pycore_pylifecycle.h" #include "pycore_pystate.h" #include "structmember.h" /* offsetof */ #include "pythread.h" @@ -11,6 +12,7 @@ static PyObject *ThreadError; static PyObject *str_dict; _Py_IDENTIFIER(stderr); +_Py_IDENTIFIER(flush); /* Lock objects */ @@ -1309,6 +1311,147 @@ requiring allocation in multiples of the system memory page size\n\ (4 KiB pages are common; using multiples of 4096 for the stack size is\n\ the suggested approach in the absence of more specific information)."); +static int +thread_excepthook_file(PyObject *file, PyObject *exc_type, PyObject *exc_value, + PyObject *exc_traceback, PyObject *thread) +{ + /* print(f"Exception in thread {thread.name}:", file=file) */ + if (PyFile_WriteString("Exception in thread ", file) < 0) { + return -1; + } + + PyObject *name = NULL; + if (thread != Py_None) { + name = PyObject_GetAttrString(thread, "name"); + } + if (name != NULL) { + if (PyFile_WriteObject(name, file, Py_PRINT_RAW) < 0) { + Py_DECREF(name); + return -1; + } + Py_DECREF(name); + } + else { + PyErr_Clear(); + + unsigned long ident = PyThread_get_thread_ident(); + PyObject *str = PyUnicode_FromFormat("%lu", ident); + if (str != NULL) { + if (PyFile_WriteObject(str, file, Py_PRINT_RAW) < 0) { + Py_DECREF(str); + return -1; + } + Py_DECREF(str); + } + else { + PyErr_Clear(); + + if (PyFile_WriteString("", file) < 0) { + return -1; + } + } + } + + if (PyFile_WriteString(":\n", file) < 0) { + return -1; + } + + /* Display the traceback */ + _PyErr_Display(file, exc_type, exc_value, exc_traceback); + + /* Call file.flush() */ + PyObject *res = _PyObject_CallMethodId(file, &PyId_flush, NULL); + if (!res) { + return -1; + } + Py_DECREF(res); + + return 0; +} + + +PyDoc_STRVAR(ExceptHookArgs__doc__, +"ExceptHookArgs\n\ +\n\ +Type used to pass arguments to threading.excepthook."); + +static PyTypeObject ExceptHookArgsType; + +static PyStructSequence_Field ExceptHookArgs_fields[] = { + {"exc_type", "Exception type"}, + {"exc_value", "Exception value"}, + {"exc_traceback", "Exception traceback"}, + {"thread", "Thread"}, + {0} +}; + +static PyStructSequence_Desc ExceptHookArgs_desc = { + .name = "_thread.ExceptHookArgs", + .doc = ExceptHookArgs__doc__, + .fields = ExceptHookArgs_fields, + .n_in_sequence = 4 +}; + + +static PyObject * +thread_excepthook(PyObject *self, PyObject *args) +{ + if (Py_TYPE(args) != &ExceptHookArgsType) { + PyErr_SetString(PyExc_TypeError, + "_thread.excepthook argument type " + "must be ExceptHookArgs"); + return NULL; + } + + /* Borrowed reference */ + PyObject *exc_type = PyStructSequence_GET_ITEM(args, 0); + if (exc_type == PyExc_SystemExit) { + /* silently ignore SystemExit */ + Py_RETURN_NONE; + } + + /* Borrowed references */ + PyObject *exc_value = PyStructSequence_GET_ITEM(args, 1); + PyObject *exc_tb = PyStructSequence_GET_ITEM(args, 2); + PyObject *thread = PyStructSequence_GET_ITEM(args, 3); + + PyObject *file = _PySys_GetObjectId(&PyId_stderr); + if (file == NULL || file == Py_None) { + if (thread == Py_None) { + /* do nothing if sys.stderr is None and thread is None */ + Py_RETURN_NONE; + } + + file = PyObject_GetAttrString(thread, "_stderr"); + if (file == NULL) { + return NULL; + } + if (file == Py_None) { + Py_DECREF(file); + /* do nothing if sys.stderr is None and sys.stderr was None + when the thread was created */ + Py_RETURN_NONE; + } + } + else { + Py_INCREF(file); + } + + int res = thread_excepthook_file(file, exc_type, exc_value, exc_tb, + thread); + Py_DECREF(file); + if (res < 0) { + return NULL; + } + + Py_RETURN_NONE; +} + +PyDoc_STRVAR(excepthook_doc, +"excepthook(exc_type, exc_value, exc_traceback, thread)\n\ +\n\ +Handle uncaught Thread.run() exception."); + static PyMethodDef thread_methods[] = { {"start_new_thread", (PyCFunction)thread_PyThread_start_new_thread, METH_VARARGS, start_new_doc}, @@ -1336,6 +1479,8 @@ static PyMethodDef thread_methods[] = { METH_VARARGS, stack_size_doc}, {"_set_sentinel", thread__set_sentinel, METH_NOARGS, _set_sentinel_doc}, + {"_excepthook", thread_excepthook, + METH_O, excepthook_doc}, {NULL, NULL} /* sentinel */ }; @@ -1388,6 +1533,12 @@ PyInit__thread(void) return NULL; if (PyType_Ready(&RLocktype) < 0) return NULL; + if (ExceptHookArgsType.tp_name == NULL) { + if (PyStructSequence_InitType2(&ExceptHookArgsType, + &ExceptHookArgs_desc) < 0) { + return NULL; + } + } /* Create the module and add the functions */ m = PyModule_Create(&threadmodule); @@ -1424,6 +1575,11 @@ PyInit__thread(void) if (PyModule_AddObject(m, "_local", (PyObject *)&localtype) < 0) return NULL; + Py_INCREF(&ExceptHookArgsType); + if (PyModule_AddObject(m, "_ExceptHookArgs", + (PyObject *)&ExceptHookArgsType) < 0) + return NULL; + interp->num_threads = 0; str_dict = PyUnicode_InternFromString("__dict__"); diff --git a/Python/pythonrun.c b/Python/pythonrun.c index 665c9c9..ba1d1cf 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -953,10 +953,11 @@ print_exception_recursive(PyObject *f, PyObject *value, PyObject *seen) } void -PyErr_Display(PyObject *exception, PyObject *value, PyObject *tb) +_PyErr_Display(PyObject *file, PyObject *exception, PyObject *value, PyObject *tb) { + assert(file != NULL && file != Py_None); + PyObject *seen; - PyObject *f = _PySys_GetObjectId(&PyId_stderr); if (PyExceptionInstance_Check(value) && tb != NULL && PyTraceBack_Check(tb)) { /* Put the traceback on the exception, otherwise it won't get @@ -967,23 +968,32 @@ PyErr_Display(PyObject *exception, PyObject *value, PyObject *tb) else Py_DECREF(cur_tb); } - if (f == Py_None) { - /* pass */ + + /* We choose to ignore seen being possibly NULL, and report + at least the main exception (it could be a MemoryError). + */ + seen = PySet_New(NULL); + if (seen == NULL) { + PyErr_Clear(); } - else if (f == NULL) { + print_exception_recursive(file, value, seen); + Py_XDECREF(seen); +} + +void +PyErr_Display(PyObject *exception, PyObject *value, PyObject *tb) +{ + PyObject *file = _PySys_GetObjectId(&PyId_stderr); + if (file == NULL) { _PyObject_Dump(value); fprintf(stderr, "lost sys.stderr\n"); + return; } - else { - /* We choose to ignore seen being possibly NULL, and report - at least the main exception (it could be a MemoryError). - */ - seen = PySet_New(NULL); - if (seen == NULL) - PyErr_Clear(); - print_exception_recursive(f, value, seen); - Py_XDECREF(seen); + if (file == Py_None) { + return; } + + _PyErr_Display(file, exception, value, tb); } PyObject * -- cgit v0.12