Skip to content

Commit

Permalink
Remove thread join from threadexception plugin (pytest-dev#13027)
Browse files Browse the repository at this point in the history
As per review comment: pytest-dev#13016 (comment).

Closes pytest-dev#13028.

---------

Co-authored-by: Bruno Oliveira <[email protected]>
  • Loading branch information
graingert and nicoddemus authored Dec 5, 2024
1 parent ea1a79a commit 80da427
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 21 deletions.
1 change: 0 additions & 1 deletion changelog/13016.improvement.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
A number of :ref:`threadexception <unraisable>` enhancements:

* Set the excepthook as early as possible and unset it as late as possible, to collect the most possible number of unhandled exceptions from threads.
* join threads for 1 second just before unsetting the excepthook, to collect any straggling exceptions
* Collect multiple thread exceptions per test phase.
* Report the :mod:`tracemalloc` allocation traceback (if available).
* Avoid using a generator based hook to allow handling :class:`StopIteration` in test failures.
Expand Down
21 changes: 3 additions & 18 deletions src/_pytest/threadexception.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import functools
import sys
import threading
import time
import traceback
from typing import NamedTuple
from typing import TYPE_CHECKING
Expand All @@ -25,22 +24,6 @@
from exceptiongroup import ExceptionGroup


def join_threads() -> None:
start = time.monotonic()
current_thread = threading.current_thread()
# This function is executed right at the end of the pytest run, just
# before we return an exit code, which is where the interpreter joins
# any remaining non-daemonic threads anyway, so it's ok to join all the
# threads. However there might be threads that depend on some shutdown
# signal that happens after pytest finishes, so we want to limit the
# join time somewhat. A one second timeout seems reasonable.
timeout = 1
for thread in threading.enumerate():
if thread is not current_thread and not thread.daemon:
# TODO: raise an error/warning if there's dangling threads.
thread.join(timeout - (time.monotonic() - start))


class ThreadExceptionMeta(NamedTuple):
msg: str
cause_msg: str
Expand Down Expand Up @@ -96,7 +79,9 @@ def cleanup(
) -> None:
try:
try:
join_threads()
# We don't join threads here, so exceptions raised from any
# threads still running by the time _threading_atexits joins them
# do not get captured (see #13027).
collect_thread_exception(config)
finally:
threading.excepthook = prev_hook
Expand Down
3 changes: 1 addition & 2 deletions testing/test_threadexception.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,16 @@ def test_2(): pass
def test_unhandled_thread_exception_after_teardown(pytester: Pytester) -> None:
pytester.makepyfile(
test_it="""
import time
import threading
import pytest
def thread():
def oops():
time.sleep(0.5)
raise ValueError("Oops")
t = threading.Thread(target=oops, name="MyThread")
t.start()
t.join()
def test_it(request):
request.config.add_cleanup(thread)
Expand Down

0 comments on commit 80da427

Please sign in to comment.