From 60a0ba6bc29d8e6daa1d54288db7fd95573dcd6b Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 20 Feb 2025 17:57:05 +0100 Subject: [PATCH 1/5] gh-87135: Raise PythonFinalizationError when joining a blocked daemon thread If `Py_IsFinalizing()` is true, non-daemon threads (other than the current one) are done, and daemon threads are prevented from running, so they cannot finalize themselves and become done. Joining them without timeout would block forever. Raise PythonFinalizationError instead of hanging. See gh-123940 for a use case: calling `join()` from `__del__`. This is ill-advised, but an exception should at least make it easier to diagnose. --- Doc/c-api/init.rst | 2 +- Doc/library/exceptions.rst | 5 ++ Doc/library/threading.rst | 11 +++++ Lib/test/test_threading.py | 93 ++++++++++++++++++++++++++++++++++++++ Modules/_threadmodule.c | 60 ++++++++++++++++-------- 5 files changed, 150 insertions(+), 21 deletions(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 9197f704fab344..3412d898a3a487 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -1112,7 +1112,7 @@ Cautions regarding runtime finalization In the late stage of :term:`interpreter shutdown`, after attempting to wait for non-daemon threads to exit (though this can be interrupted by :class:`KeyboardInterrupt`) and running the :mod:`atexit` functions, the runtime -is marked as *finalizing*: :c:func:`_Py_IsFinalizing` and +is marked as *finalizing*: :c:func:`Py_IsFinalizing` and :func:`sys.is_finalizing` return true. At this point, only the *finalization thread* that initiated finalization (typically the main thread) is allowed to acquire the :term:`GIL`. diff --git a/Doc/library/exceptions.rst b/Doc/library/exceptions.rst index 319d261ef3fb4d..a7febd736d0196 100644 --- a/Doc/library/exceptions.rst +++ b/Doc/library/exceptions.rst @@ -426,6 +426,8 @@ The following exceptions are the exceptions that are usually raised. :exc:`PythonFinalizationError` during the Python finalization: * Creating a new Python thread. + * :meth:`Joining ` a running daemon thread + without a timeout. * :func:`os.fork`. See also the :func:`sys.is_finalizing` function. @@ -433,6 +435,9 @@ The following exceptions are the exceptions that are usually raised. .. versionadded:: 3.13 Previously, a plain :exc:`RuntimeError` was raised. + .. versionchanged:: next + + :meth:`threading.Thread.join` can now raise this exception. .. exception:: RecursionError diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 00511df32e4388..11f8594be4f06c 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -435,6 +435,17 @@ since it is impossible to detect the termination of alien threads. an error to :meth:`~Thread.join` a thread before it has been started and attempts to do so raise the same exception. + In late stages of :term:`Python finalization `, + if *timeout* is ``None`` and an attempt is made to join a running + daemonic thread, :meth:`!join` raises a :exc:`PythonFinalizationError`. + (Such a join would block forever: at this point, threads other than the + current one are prevented from running Python code and so they cannot + finalize themselves.) + + .. versionchanged:: next + + May raise :exc:`PythonFinalizationError`. + .. attribute:: name A string used for identification purposes only. It has no semantics. diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 214e1ba0b53dd2..c33e7cdad683a6 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1171,6 +1171,99 @@ def __del__(self): self.assertEqual(out.strip(), b"OK") self.assertIn(b"can't create new thread at interpreter shutdown", err) + def test_join_daemon_thread_in_finalization(self): + # gh-123940: Py_Finalize() prevents other threads from running Python + # code, so join() can not succeed unless the thread is already done. + # (Non-Python threads, that is `threading._DummyThread`, can't be + # joined at all.) + # We raise an exception rather than hang. + code = textwrap.dedent(""" + import threading + + + def loop(): + while True: + pass + + + class Cycle: + def __init__(self): + self.self_ref = self + self.thr = threading.Thread(target=loop, daemon=True) + self.thr.start() + + def __del__(self): + try: + self.thr.join() + except PythonFinalizationError: + print('got the correct exception!') + + # Cycle holds a reference to itself, which ensures it is cleaned + # up during the GC that runs after daemon threads have been + # forced to exit during finalization. + Cycle() + """) + rc, out, err = assert_python_ok("-c", code) + self.assertEqual(err, b"") + self.assertIn(b"got the correct exception", out) + + def test_join_finished_daemon_thread_in_finalization(self): + # (see previous test) + # If the thread is already finished, join() succeeds. + code = textwrap.dedent(""" + import threading + done = threading.Event() + + def loop(): + done.set() + + + class Cycle: + def __init__(self): + self.self_ref = self + self.thr = threading.Thread(target=loop, daemon=True) + self.thr.start() + done.wait() + + def __del__(self): + self.thr.join() + print('all clear!') + + Cycle() + """) + rc, out, err = assert_python_ok("-c", code) + self.assertEqual(err, b"") + self.assertIn(b"all clear", out) + + def test_timed_join_daemon_thread_in_finalization(self): + # (see previous test) + # When called with timeout, no error is raised. + code = textwrap.dedent(""" + import threading + done = threading.Event() + + def loop(): + done.set() + while True: + pass + + class Cycle: + def __init__(self): + self.self_ref = self + self.thr = threading.Thread(target=loop, daemon=True) + self.thr.start() + done.wait() + + def __del__(self): + self.thr.join(timeout=0.01) + print('alive:', self.thr.is_alive()) + + Cycle() + """) + rc, out, err = assert_python_ok("-c", code) + self.assertEqual(err, b"") + self.assertIn(b"alive: True", out) + def test_start_new_thread_failed(self): # gh-109746: if Python fails to start newly created thread # due to failure of underlying PyThread_start_new_thread() call, diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index e251736fb36aa9..5d1af748600338 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -510,32 +510,52 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) // To work around this, we set `thread_is_exiting` immediately before // `thread_run` returns. We can be sure that we are not attempting to join // ourselves if the handle's thread is about to exit. - if (!_PyEvent_IsSet(&self->thread_is_exiting) && - ThreadHandle_ident(self) == PyThread_get_thread_ident_ex()) { - // PyThread_join_thread() would deadlock or error out. - PyErr_SetString(ThreadError, "Cannot join current thread"); - return -1; - } - - // Wait until the deadline for the thread to exit. - PyTime_t deadline = timeout_ns != -1 ? _PyDeadline_Init(timeout_ns) : 0; - int detach = 1; - while (!PyEvent_WaitTimed(&self->thread_is_exiting, timeout_ns, detach)) { - if (deadline) { - // _PyDeadline_Get will return a negative value if the deadline has - // been exceeded. - timeout_ns = Py_MAX(_PyDeadline_Get(deadline), 0); + PyEvent *is_exiting = &self->thread_is_exiting; + if (!_PyEvent_IsSet(is_exiting)) { + if (ThreadHandle_ident(self) == PyThread_get_thread_ident_ex()) { + // PyThread_join_thread() would deadlock or error out. + PyErr_SetString(ThreadError, "Cannot join current thread"); + return -1; } - if (timeout_ns) { - // Interrupted - if (Py_MakePendingCalls() < 0) { + PyTime_t deadline = 0; + + if (timeout_ns == -1) { + if (Py_IsFinalizing()) { + // gh-123940: On finalization, other threads are prevented from + // running Python code. They cannot finalize themselves, + // so join() would hang forever. + // We raise instead. + // (We only do this if no timeout is given: otherwise + // we assume the caller can handle a hung thread.) + PyErr_SetString(PyExc_PythonFinalizationError, + "cannot join thread at interpreter shutdown"); return -1; } } else { - // Timed out - return 0; + deadline = _PyDeadline_Init(timeout_ns); + } + + // Wait until the deadline for the thread to exit. + int detach = 1; + while (!PyEvent_WaitTimed(is_exiting, timeout_ns, detach)) { + if (deadline) { + // _PyDeadline_Get will return a negative value if + // the deadline has been exceeded. + timeout_ns = Py_MAX(_PyDeadline_Get(deadline), 0); + } + + if (timeout_ns) { + // Interrupted + if (Py_MakePendingCalls() < 0) { + return -1; + } + } + else { + // Timed out + return 0; + } } } From 549d04a3970d72e55c1febfddecc8e4f7770631d Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 21 Feb 2025 15:46:48 +0100 Subject: [PATCH 2/5] Add blurb --- .../next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst diff --git a/Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst b/Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst new file mode 100644 index 00000000000000..6a8b1e38537af2 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst @@ -0,0 +1,2 @@ +Joining running daemon threads during interpreter shutdown without timeout +now raises :exc:`PythonFinalizationError`. From 4aecdf5ffdfac56ae2c9520add06af721812a81f Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 27 Feb 2025 16:58:06 +0100 Subject: [PATCH 3/5] Raise for calls with timeout too --- Doc/library/exceptions.rst | 3 +- Doc/library/threading.rst | 9 +- Lib/test/test_threading.py | 90 +++++++------------ ...-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst | 2 +- Modules/_threadmodule.c | 29 +++--- 5 files changed, 48 insertions(+), 85 deletions(-) diff --git a/Doc/library/exceptions.rst b/Doc/library/exceptions.rst index a7febd736d0196..1ca3888ad31c8e 100644 --- a/Doc/library/exceptions.rst +++ b/Doc/library/exceptions.rst @@ -426,8 +426,7 @@ The following exceptions are the exceptions that are usually raised. :exc:`PythonFinalizationError` during the Python finalization: * Creating a new Python thread. - * :meth:`Joining ` a running daemon thread - without a timeout. + * :meth:`Joining ` a running daemon thread. * :func:`os.fork`. See also the :func:`sys.is_finalizing` function. diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index 11f8594be4f06c..52f3bdf71c0a02 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -435,12 +435,9 @@ since it is impossible to detect the termination of alien threads. an error to :meth:`~Thread.join` a thread before it has been started and attempts to do so raise the same exception. - In late stages of :term:`Python finalization `, - if *timeout* is ``None`` and an attempt is made to join a running - daemonic thread, :meth:`!join` raises a :exc:`PythonFinalizationError`. - (Such a join would block forever: at this point, threads other than the - current one are prevented from running Python code and so they cannot - finalize themselves.) + If an attempt is made to join a running daemonic thread in in late stages + of :term:`Python finalization ` :meth:`!join` + raises a :exc:`PythonFinalizationError`. .. versionchanged:: next diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index c33e7cdad683a6..2f98b1f181ee9f 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1177,35 +1177,38 @@ def test_join_daemon_thread_in_finalization(self): # (Non-Python threads, that is `threading._DummyThread`, can't be # joined at all.) # We raise an exception rather than hang. - code = textwrap.dedent(""" - import threading - - - def loop(): - while True: - pass - - - class Cycle: - def __init__(self): - self.self_ref = self - self.thr = threading.Thread(target=loop, daemon=True) - self.thr.start() - - def __del__(self): - try: - self.thr.join() - except PythonFinalizationError: - print('got the correct exception!') - - # Cycle holds a reference to itself, which ensures it is cleaned - # up during the GC that runs after daemon threads have been - # forced to exit during finalization. - Cycle() - """) - rc, out, err = assert_python_ok("-c", code) - self.assertEqual(err, b"") - self.assertIn(b"got the correct exception", out) + for timeout in (None, 10): + with self.subTest(timeout=timeout): + code = textwrap.dedent(f""" + import threading + + + def loop(): + while True: + pass + + + class Cycle: + def __init__(self): + self.self_ref = self + self.thr = threading.Thread( + target=loop, daemon=True) + self.thr.start() + + def __del__(self): + try: + self.thr.join(timeout={timeout}) + except PythonFinalizationError: + print('got the correct exception!') + + # Cycle holds a reference to itself, which ensures it is + # cleaned up during the GC that runs after daemon threads + # have been forced to exit during finalization. + Cycle() + """) + rc, out, err = assert_python_ok("-c", code) + self.assertEqual(err, b"") + self.assertIn(b"got the correct exception", out) def test_join_finished_daemon_thread_in_finalization(self): # (see previous test) @@ -1235,35 +1238,6 @@ def __del__(self): self.assertEqual(err, b"") self.assertIn(b"all clear", out) - def test_timed_join_daemon_thread_in_finalization(self): - # (see previous test) - # When called with timeout, no error is raised. - code = textwrap.dedent(""" - import threading - done = threading.Event() - - def loop(): - done.set() - while True: - pass - - class Cycle: - def __init__(self): - self.self_ref = self - self.thr = threading.Thread(target=loop, daemon=True) - self.thr.start() - done.wait() - - def __del__(self): - self.thr.join(timeout=0.01) - print('alive:', self.thr.is_alive()) - - Cycle() - """) - rc, out, err = assert_python_ok("-c", code) - self.assertEqual(err, b"") - self.assertIn(b"alive: True", out) - def test_start_new_thread_failed(self): # gh-109746: if Python fails to start newly created thread # due to failure of underlying PyThread_start_new_thread() call, diff --git a/Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst b/Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst index 6a8b1e38537af2..b91d429f3f7bf6 100644 --- a/Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst +++ b/Misc/NEWS.d/next/Library/2025-02-21-15-46-43.gh-issue-130402.Rwu_KK.rst @@ -1,2 +1,2 @@ -Joining running daemon threads during interpreter shutdown without timeout +Joining running daemon threads during interpreter shutdown now raises :exc:`PythonFinalizationError`. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index df34812c96555d..c1c99e5c3b71e7 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -517,27 +517,20 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) PyErr_SetString(ThreadError, "Cannot join current thread"); return -1; } - - PyTime_t deadline = 0; - - if (timeout_ns == -1) { - if (Py_IsFinalizing()) { - // gh-123940: On finalization, other threads are prevented from - // running Python code. They cannot finalize themselves, - // so join() would hang forever. - // We raise instead. - // (We only do this if no timeout is given: otherwise - // we assume the caller can handle a hung thread.) - PyErr_SetString(PyExc_PythonFinalizationError, - "cannot join thread at interpreter shutdown"); - return -1; - } - } - else { - deadline = _PyDeadline_Init(timeout_ns); + if (Py_IsFinalizing()) { + // gh-123940: On finalization, other threads are prevented from + // running Python code. They cannot finalize themselves, + // so join() would hang forever. + // We raise instead. + // (We only do this if no timeout is given: otherwise + // we assume the caller can handle a hung thread.) + PyErr_SetString(PyExc_PythonFinalizationError, + "cannot join thread at interpreter shutdown"); + return -1; } // Wait until the deadline for the thread to exit. + PyTime_t deadline = timeout_ns != -1 ? _PyDeadline_Init(timeout_ns) : 0; int detach = 1; while (!PyEvent_WaitTimed(is_exiting, timeout_ns, detach)) { if (deadline) { From e463cb8fb37c3afb27b18415518cee8d52165386 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 27 Feb 2025 16:59:21 +0100 Subject: [PATCH 4/5] Add asserts for is_alive() --- Lib/test/test_threading.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 2f98b1f181ee9f..cb5686963150db 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1196,9 +1196,11 @@ def __init__(self): self.thr.start() def __del__(self): + assert self.thr.is_alive() try: self.thr.join(timeout={timeout}) except PythonFinalizationError: + assert self.thr.is_alive() print('got the correct exception!') # Cycle holds a reference to itself, which ensures it is @@ -1229,7 +1231,9 @@ def __init__(self): done.wait() def __del__(self): + assert not self.thr.is_alive() self.thr.join() + assert not self.thr.is_alive() print('all clear!') Cycle() From d99b42b24e3ab8c4cee3c29493cf6ce6312f4859 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 27 Feb 2025 17:07:57 +0100 Subject: [PATCH 5/5] Reduce diff --- Modules/_threadmodule.c | 43 +++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index c1c99e5c3b71e7..802bd7c4feb0b1 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -510,8 +510,7 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) // To work around this, we set `thread_is_exiting` immediately before // `thread_run` returns. We can be sure that we are not attempting to join // ourselves if the handle's thread is about to exit. - PyEvent *is_exiting = &self->thread_is_exiting; - if (!_PyEvent_IsSet(is_exiting)) { + if (!_PyEvent_IsSet(&self->thread_is_exiting)) { if (ThreadHandle_ident(self) == PyThread_get_thread_ident_ex()) { // PyThread_join_thread() would deadlock or error out. PyErr_SetString(ThreadError, "Cannot join current thread"); @@ -520,36 +519,34 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) if (Py_IsFinalizing()) { // gh-123940: On finalization, other threads are prevented from // running Python code. They cannot finalize themselves, - // so join() would hang forever. + // so join() would hang forever (or until timeout). // We raise instead. - // (We only do this if no timeout is given: otherwise - // we assume the caller can handle a hung thread.) PyErr_SetString(PyExc_PythonFinalizationError, "cannot join thread at interpreter shutdown"); return -1; } + } - // Wait until the deadline for the thread to exit. - PyTime_t deadline = timeout_ns != -1 ? _PyDeadline_Init(timeout_ns) : 0; - int detach = 1; - while (!PyEvent_WaitTimed(is_exiting, timeout_ns, detach)) { - if (deadline) { - // _PyDeadline_Get will return a negative value if - // the deadline has been exceeded. - timeout_ns = Py_MAX(_PyDeadline_Get(deadline), 0); - } + // Wait until the deadline for the thread to exit. + PyTime_t deadline = timeout_ns != -1 ? _PyDeadline_Init(timeout_ns) : 0; + int detach = 1; + while (!PyEvent_WaitTimed(&self->thread_is_exiting, timeout_ns, detach)) { + if (deadline) { + // _PyDeadline_Get will return a negative value if the deadline has + // been exceeded. + timeout_ns = Py_MAX(_PyDeadline_Get(deadline), 0); + } - if (timeout_ns) { - // Interrupted - if (Py_MakePendingCalls() < 0) { - return -1; - } - } - else { - // Timed out - return 0; + if (timeout_ns) { + // Interrupted + if (Py_MakePendingCalls() < 0) { + return -1; } } + else { + // Timed out + return 0; + } } if (_PyOnceFlag_CallOnce(&self->once, (_Py_once_fn_t *)join_thread,