_thread.start_new_thread(): call sys.unraisablehook() to handle uncaught exceptions

[vstinner]

BPO	37076
Nosy	@vstinner, @serhiy-storchaka, @lazka
PRs	bpo-37076: _thread.start_new_thread() calls _PyErr_WriteUnraisableMsg() #13617

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-05-29.01:03:33.867>
created_at = <Date 2019-05-28.10:15:10.365>
labels = ['3.8', 'library']
title = '_thread.start_new_thread(): call sys.unraisablehook() to handle uncaught exceptions'
updated_at = <Date 2019-05-29.01:03:33.866>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2019-05-29.01:03:33.866>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-29.01:03:33.867>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2019-05-28.10:15:10.365>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 37076
keywords = ['patch']
message_count = 10.0
messages = ['343756', '343757', '343759', '343788', '343805', '343806', '343815', '343823', '343845', '343846']
nosy_count = 3.0
nosy_names = ['vstinner', 'serhiy.storchaka', 'lazka']
pr_nums = ['13617']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue37076'
versions = ['Python 3.8']

[vstinner]

Python 3.8 now has 3 hooks for uncaught exceptions:

• sys.excepthook()
• sys.unraisablehook()
• threading.excepthook()

_thread.start_new_thread() calls none of these hooks, but directly logs the exception.

I propose to modify it to reuse sys.unraisablehook(): see attached PR.

--

Using threading.excepthook() would be another option, but _thread.start_new_thread() is usually wrapped in threading.Thread.run() which already uses threading.excepthook(). It might be surprising to see two bugs from the same thread using threading.excepthook().

Moreover, right now, _thread is the "low-level" API to access threads: it doesn't acces threading. I'm not comfortable by adding an inter-dependency between _thread (low-level) and threading (high-level).

If threading.Thread.run() is correctly written, it should not raise an exception. In the worst case, threading.excepthook should handle the exception. _thread.start_new_thread() should never get an exception raised by threading if threading.excepthook does correctly its job.

[vstinner]

With PR 13617, the function which raised the exception can be retrieved using unraisable.object.

If we used threading.excepthook, I don't see how we could pass this function into threading.ExceptHookArgs: ExceptHookArgs.thread is expected to be a threading.Thread object.

[vstinner]

This issue is a follow-up of:

• bpo-36829: Add sys.unraisablehook() to custom how "unraisable exceptions" are logged
• bpo-1230540: Add threading.excepthook() to handle uncaught exceptions raised by Thread.run()

Discussions around _thread.start_new_thead() exception:

Antoine Pitrou:
"""
From the doc: Handle uncaught :func:``Thread.run`` exception.

What if some C extension wants to report uncaught exceptions in C-created threads? Does it have to create a dummy Thread object?

For example, how about threads created by _thread.start_new_thread?
"""

#13515 (comment)

Serhiy Storchaka:
"If we want to support low-level threads created by start_new_thread() we should call [threading.]excepthook() from t_bootstrap instead of Thread._bootstrap_inner. Otherwise implementing excepthook() as a Thread method would be more convenient."

https://bugs.python.org/issue1230540#msg343748

[lazka]

_thread.start_new_thread() calls none of these hooks, but directly logs the exception.

It calls sys.excepthook() currently:

import _thread
import threading
import sys

done = False
def hook(*args):
    global done
    print(threading.current_thread())
    done = True
sys.excepthook = hook

def worker():
    raise Exception
_thread.start_new_thread(worker, tuple())
while not done:
    pass

[vstinner]

Output:
---
Unhandled exception in thread started by <function worker at 0x7fae27f212d0>
<_DummyThread(Dummy-1, started daemon 140385971111680)>
---

Ah right, sys.excepthook is called! But "Unhandled exception in thread started by <function worker at 0x7fae27f212d0>" line is always written into stderr, it cannot be avoided :-( And sys.excepthook doesn't provide access to the thread function which caused the issue.

[vstinner]

I updated my PR description:

_thread.start_new_thread() now logs uncaught exception raised by the
function using sys.unraisablehook(), rather than sys.excepthook(), so
the hook gets access to the function which raised the exception.

[serhiy-storchaka]

If threading.Thread.run() is correctly written, it should not raise an exception.

Since the error handling for threading.Thread.run() is written on Python there are many opportunities to get an exception in error handling: KeyboardInterrupt, MemoryError and, at the shutdown stage, maybe NameError, AttributeError or TypeError.

[vstinner]

Since the error handling for threading.Thread.run() is written on Python there are many opportunities to get an exception in error handling: KeyboardInterrupt, MemoryError and, at the shutdown stage, maybe NameError, AttributeError or TypeError.

Sure, the risk is real, but I tried to minimize it.

NameError and AttributeError "should" not happen: I wrote _make_invoke_excepthook() to prevent name errors. Functions used to invoke threading.excepthook are "cached" in a private namespace. Moreover, the default hook is implemented in C also to reduce the risk of raising a new exception.

Anyway, if threading.excepthook() raises a second exception, sys.excepthook() is now called to handle it ;-) That's a Python 3.8 change. In Python 3.7 and older, new exception was handled by start_new_thread().

[vstinner]

New changeset 8b09500 by Victor Stinner in branch 'master':
bpo-37076: _thread.start_new_thread() calls _PyErr_WriteUnraisableMsg() (GH-13617)
8b09500

[vstinner]

Thanks Serhiy for the review!