Skip to content

Commit 469849e

Browse files
author
Rémy Voet (ryv)
committed
gh-140746: Fix Thread.start() that can hang indefinitely
When a new thread is started (`Thread.start()`), the current thread waits for the new thread's `_started` signal (`self._started.wait()`). If the new thread doesn't have enough memory, it might not be able to signal that it started to the parent thread, and the parent thread will wait indefinitely. To fix this, we reintroduce the old timeout for `Thread.start()` that was used before commit 9e7f1d2. When the timeout occurs, we check the `_os_thread_handle` to see if the thread has somehow died. If it has, we can ensure the thread has been killed without it having sent the start signal. We also change `Thread._delete()` to use `pop` instead, because nothing guarantees that the thread exists in `_active[get_ident()]` (to avoid KeyError). `_bootstrap_inner` might crash before `_active[self._ident] = self` executes. We also use `self._ident` because we know that `set_ident` has already been executed. Remove the old comment of `_delete` because `_active_limbo_lock` has became reentrant since 243fd01
1 parent dbe3950 commit 469849e

File tree

2 files changed

+35
-6
lines changed

2 files changed

+35
-6
lines changed

Lib/test/test_threading.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,32 @@ def run_in_bg():
14571457
self.assertEqual(err, b"")
14581458
self.assertEqual(out.strip(), b"Exiting...")
14591459

1460+
def test_memory_error_bootstrap(self):
1461+
# gh-140746: Test that Thread.start() doesn't hang indefinitely if
1462+
# the new thread fails (MemoryError) during its initialization
1463+
1464+
def serving_thread():
1465+
1466+
def nothing():
1467+
pass
1468+
1469+
def _set_ident_memory_error():
1470+
raise MemoryError()
1471+
1472+
thread = threading.Thread(target=nothing)
1473+
with (
1474+
support.catch_unraisable_exception(),
1475+
mock.patch.object(thread, '_set_ident', _set_ident_memory_error)
1476+
):
1477+
thread.start()
1478+
self.assertFalse(thread in threading._limbo)
1479+
self.assertFalse(thread.is_alive())
1480+
1481+
serving_thread = threading.Thread(target=serving_thread)
1482+
serving_thread.start()
1483+
serving_thread.join(0.1)
1484+
self.assertFalse(serving_thread.is_alive())
1485+
14601486
class ThreadJoinOnShutdown(BaseTestCase):
14611487

14621488
def _run_and_join(self, script):

Lib/threading.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,7 +1001,14 @@ def start(self):
10011001
with _active_limbo_lock:
10021002
del _limbo[self]
10031003
raise
1004-
self._started.wait() # Will set ident and native_id
1004+
# It's possible that the _started event never occurs from the new Thread;
1005+
# e.g., it didn't have enough memory to call the initialization part of _bootstrap_inner.
1006+
while not self._started.wait(0.000001):
1007+
if self._os_thread_handle.is_done():
1008+
self._started.set()
1009+
with _active_limbo_lock:
1010+
_limbo.pop(self, None)
1011+
break
10051012

10061013
def run(self):
10071014
"""Method representing the thread's activity.
@@ -1081,11 +1088,7 @@ def _bootstrap_inner(self):
10811088
def _delete(self):
10821089
"Remove current thread from the dict of currently running threads."
10831090
with _active_limbo_lock:
1084-
del _active[get_ident()]
1085-
# There must not be any python code between the previous line
1086-
# and after the lock is released. Otherwise a tracing function
1087-
# could try to acquire the lock again in the same thread, (in
1088-
# current_thread()), and would block.
1091+
_active.pop(self._ident, None)
10891092

10901093
def join(self, timeout=None):
10911094
"""Wait until the thread terminates.

0 commit comments

Comments
 (0)