Skip to content

Commit 5a1ecc8

Browse files
authored
gh-114423: Remove DummyThread from threading._active when thread dies (GH-114424)
1 parent 7d21cae commit 5a1ecc8

File tree

3 files changed

+76
-28
lines changed

3 files changed

+76
-28
lines changed

Lib/test/test_threading.py

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,6 @@ def f():
227227
tid = _thread.start_new_thread(f, ())
228228
done.wait()
229229
self.assertEqual(ident[0], tid)
230-
# Kill the "immortal" _DummyThread
231-
del threading._active[ident[0]]
232230

233231
# run with a small(ish) thread stack size (256 KiB)
234232
def test_various_ops_small_stack(self):
@@ -256,32 +254,55 @@ def test_various_ops_large_stack(self):
256254

257255
def test_foreign_thread(self):
258256
# Check that a "foreign" thread can use the threading module.
257+
dummy_thread = None
258+
error = None
259259
def f(mutex):
260-
# Calling current_thread() forces an entry for the foreign
261-
# thread to get made in the threading._active map.
262-
threading.current_thread()
263-
mutex.release()
260+
try:
261+
nonlocal dummy_thread
262+
nonlocal error
263+
# Calling current_thread() forces an entry for the foreign
264+
# thread to get made in the threading._active map.
265+
dummy_thread = threading.current_thread()
266+
tid = dummy_thread.ident
267+
self.assertIn(tid, threading._active)
268+
self.assertIsInstance(dummy_thread, threading._DummyThread)
269+
self.assertIs(threading._active.get(tid), dummy_thread)
270+
# gh-29376
271+
self.assertTrue(
272+
dummy_thread.is_alive(),
273+
'Expected _DummyThread to be considered alive.'
274+
)
275+
self.assertIn('_DummyThread', repr(dummy_thread))
276+
except BaseException as e:
277+
error = e
278+
finally:
279+
mutex.release()
264280

265281
mutex = threading.Lock()
266282
mutex.acquire()
267283
with threading_helper.wait_threads_exit():
268284
tid = _thread.start_new_thread(f, (mutex,))
269285
# Wait for the thread to finish.
270286
mutex.acquire()
271-
self.assertIn(tid, threading._active)
272-
self.assertIsInstance(threading._active[tid], threading._DummyThread)
273-
#Issue 29376
274-
self.assertTrue(threading._active[tid].is_alive())
275-
self.assertRegex(repr(threading._active[tid]), '_DummyThread')
276-
287+
if error is not None:
288+
raise error
289+
self.assertEqual(tid, dummy_thread.ident)
277290
# Issue gh-106236:
278291
with self.assertRaises(RuntimeError):
279-
threading._active[tid].join()
280-
threading._active[tid]._started.clear()
292+
dummy_thread.join()
293+
dummy_thread._started.clear()
281294
with self.assertRaises(RuntimeError):
282-
threading._active[tid].is_alive()
283-
284-
del threading._active[tid]
295+
dummy_thread.is_alive()
296+
# Busy wait for the following condition: after the thread dies, the
297+
# related dummy thread must be removed from threading._active.
298+
timeout = 5
299+
timeout_at = time.monotonic() + timeout
300+
while time.monotonic() < timeout_at:
301+
if threading._active.get(dummy_thread.ident) is not dummy_thread:
302+
break
303+
time.sleep(.1)
304+
else:
305+
self.fail('It was expected that the created threading._DummyThread was removed from threading._active.')
285306

286307
# PyThreadState_SetAsyncExc() is a CPython-only gimmick, not (currently)
287308
# exposed at the Python level. This test relies on ctypes to get at it.

Lib/threading.py

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@
5454
TIMEOUT_MAX = _thread.TIMEOUT_MAX
5555
del _thread
5656

57+
# get thread-local implementation, either from the thread
58+
# module, or from the python fallback
59+
60+
try:
61+
from _thread import _local as local
62+
except ImportError:
63+
from _threading_local import local
5764

5865
# Support for profile and trace hooks
5966

@@ -1476,10 +1483,36 @@ def __init__(self):
14761483
_active[self._ident] = self
14771484

14781485

1486+
# Helper thread-local instance to detect when a _DummyThread
1487+
# is collected. Not a part of the public API.
1488+
_thread_local_info = local()
1489+
1490+
1491+
class _DeleteDummyThreadOnDel:
1492+
'''
1493+
Helper class to remove a dummy thread from threading._active on __del__.
1494+
'''
1495+
1496+
def __init__(self, dummy_thread):
1497+
self._dummy_thread = dummy_thread
1498+
self._tident = dummy_thread.ident
1499+
# Put the thread on a thread local variable so that when
1500+
# the related thread finishes this instance is collected.
1501+
#
1502+
# Note: no other references to this instance may be created.
1503+
# If any client code creates a reference to this instance,
1504+
# the related _DummyThread will be kept forever!
1505+
_thread_local_info._track_dummy_thread_ref = self
1506+
1507+
def __del__(self):
1508+
with _active_limbo_lock:
1509+
if _active.get(self._tident) is self._dummy_thread:
1510+
_active.pop(self._tident, None)
1511+
1512+
14791513
# Dummy thread class to represent threads not started here.
1480-
# These aren't garbage collected when they die, nor can they be waited for.
1481-
# If they invoke anything in threading.py that calls current_thread(), they
1482-
# leave an entry in the _active dict forever after.
1514+
# These should be added to `_active` and removed automatically
1515+
# when they die, although they can't be waited for.
14831516
# Their purpose is to return *something* from current_thread().
14841517
# They are marked as daemon threads so we won't wait for them
14851518
# when we exit (conform previous semantics).
@@ -1495,6 +1528,7 @@ def __init__(self):
14951528
self._set_native_id()
14961529
with _active_limbo_lock:
14971530
_active[self._ident] = self
1531+
_DeleteDummyThreadOnDel(self)
14981532

14991533
def _stop(self):
15001534
pass
@@ -1676,14 +1710,6 @@ def main_thread():
16761710
# XXX Figure this out for subinterpreters. (See gh-75698.)
16771711
return _main_thread
16781712

1679-
# get thread-local implementation, either from the thread
1680-
# module, or from the python fallback
1681-
1682-
try:
1683-
from _thread import _local as local
1684-
except ImportError:
1685-
from _threading_local import local
1686-
16871713

16881714
def _after_fork():
16891715
"""
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
``_DummyThread`` entries in ``threading._active`` are now automatically removed when the related thread dies.

0 commit comments

Comments
 (0)