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

Activity

1. vstinner commented on May 28, 2019

   

   vstinner

   on May 28, 2019

   MemberAuthor

   More actions

   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.
2. vstinner

   added

   3.8 (EOL)end of lifeend of life

   stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory

   on May 28, 2019
3. vstinner commented on May 28, 2019

   

   vstinner

   on May 28, 2019

   MemberAuthor

   More actions

   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.
4. vstinner commented on May 28, 2019

   

   vstinner

   on May 28, 2019

   MemberAuthor

   More actions

   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
5. lazka commented on May 28, 2019

   

   lazkamannequin

   on May 28, 2019

   Mannequin

   More actions

   _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
6. vstinner commented on May 28, 2019

   

   vstinner

   on May 28, 2019

   MemberAuthor

   More actions

   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.
7. vstinner commented on May 28, 2019

   

   vstinner

   on May 28, 2019

   MemberAuthor

   More actions

   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.
8. serhiy-storchaka commented on May 28, 2019

   

   serhiy-storchaka

   on May 28, 2019

   Member

   More actions

   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.
9. vstinner commented on May 28, 2019

   

   vstinner

   on May 28, 2019

   MemberAuthor

   More actions

   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().
10. vstinner commented on May 29, 2019

    

    vstinner

    on May 29, 2019

    MemberAuthor

    More actions

    New changeset 8b09500 by Victor Stinner in branch 'master':
    bpo-37076: _thread.start_new_thread() calls _PyErr_WriteUnraisableMsg() (GH-13617)
    8b09500
11. vstinner commented on May 29, 2019

    

    vstinner

    on May 29, 2019

    MemberAuthor

    More actions

    Thanks Serhiy for the review!
12. vstinner

    closed this as completedon May 29, 2019
13. ezio-melotti

    transferred this issue fromon Apr 10, 2022