From 5a1ecc8cc7d3dfedd14adea1c3cdc3cfeb79f0e1 Mon Sep 17 00:00:00 2001 From: Fabio Zadrozny <117621+fabioz@users.noreply.github.com> Date: Tue, 23 Jan 2024 09:12:50 -0300 Subject: gh-114423: Remove DummyThread from threading._active when thread dies (GH-114424) --- Lib/test/test_threading.py | 55 +++++++++++++++------- Lib/threading.py | 48 ++++++++++++++----- .../2024-01-22-11-43-38.gh-issue-114423.6mMoPH.rst | 1 + 3 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-01-22-11-43-38.gh-issue-114423.6mMoPH.rst diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 7160c53..dbdc46f 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -227,8 +227,6 @@ class ThreadTests(BaseTestCase): tid = _thread.start_new_thread(f, ()) done.wait() self.assertEqual(ident[0], tid) - # Kill the "immortal" _DummyThread - del threading._active[ident[0]] # run with a small(ish) thread stack size (256 KiB) def test_various_ops_small_stack(self): @@ -256,11 +254,29 @@ class ThreadTests(BaseTestCase): def test_foreign_thread(self): # Check that a "foreign" thread can use the threading module. + dummy_thread = None + error = None def f(mutex): - # Calling current_thread() forces an entry for the foreign - # thread to get made in the threading._active map. - threading.current_thread() - mutex.release() + try: + nonlocal dummy_thread + nonlocal error + # Calling current_thread() forces an entry for the foreign + # thread to get made in the threading._active map. + dummy_thread = threading.current_thread() + tid = dummy_thread.ident + self.assertIn(tid, threading._active) + self.assertIsInstance(dummy_thread, threading._DummyThread) + self.assertIs(threading._active.get(tid), dummy_thread) + # gh-29376 + self.assertTrue( + dummy_thread.is_alive(), + 'Expected _DummyThread to be considered alive.' + ) + self.assertIn('_DummyThread', repr(dummy_thread)) + except BaseException as e: + error = e + finally: + mutex.release() mutex = threading.Lock() mutex.acquire() @@ -268,20 +284,25 @@ class ThreadTests(BaseTestCase): tid = _thread.start_new_thread(f, (mutex,)) # Wait for the thread to finish. mutex.acquire() - self.assertIn(tid, threading._active) - self.assertIsInstance(threading._active[tid], threading._DummyThread) - #Issue 29376 - self.assertTrue(threading._active[tid].is_alive()) - self.assertRegex(repr(threading._active[tid]), '_DummyThread') - + if error is not None: + raise error + self.assertEqual(tid, dummy_thread.ident) # Issue gh-106236: with self.assertRaises(RuntimeError): - threading._active[tid].join() - threading._active[tid]._started.clear() + dummy_thread.join() + dummy_thread._started.clear() with self.assertRaises(RuntimeError): - threading._active[tid].is_alive() - - del threading._active[tid] + dummy_thread.is_alive() + # Busy wait for the following condition: after the thread dies, the + # related dummy thread must be removed from threading._active. + timeout = 5 + timeout_at = time.monotonic() + timeout + while time.monotonic() < timeout_at: + if threading._active.get(dummy_thread.ident) is not dummy_thread: + break + time.sleep(.1) + else: + self.fail('It was expected that the created threading._DummyThread was removed from threading._active.') # PyThreadState_SetAsyncExc() is a CPython-only gimmick, not (currently) # exposed at the Python level. This test relies on ctypes to get at it. diff --git a/Lib/threading.py b/Lib/threading.py index c561800..ecf799b 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -54,6 +54,13 @@ except AttributeError: TIMEOUT_MAX = _thread.TIMEOUT_MAX del _thread +# get thread-local implementation, either from the thread +# module, or from the python fallback + +try: + from _thread import _local as local +except ImportError: + from _threading_local import local # Support for profile and trace hooks @@ -1476,10 +1483,36 @@ class _MainThread(Thread): _active[self._ident] = self +# Helper thread-local instance to detect when a _DummyThread +# is collected. Not a part of the public API. +_thread_local_info = local() + + +class _DeleteDummyThreadOnDel: + ''' + Helper class to remove a dummy thread from threading._active on __del__. + ''' + + def __init__(self, dummy_thread): + self._dummy_thread = dummy_thread + self._tident = dummy_thread.ident + # Put the thread on a thread local variable so that when + # the related thread finishes this instance is collected. + # + # Note: no other references to this instance may be created. + # If any client code creates a reference to this instance, + # the related _DummyThread will be kept forever! + _thread_local_info._track_dummy_thread_ref = self + + def __del__(self): + with _active_limbo_lock: + if _active.get(self._tident) is self._dummy_thread: + _active.pop(self._tident, None) + + # Dummy thread class to represent threads not started here. -# These aren't garbage collected when they die, nor can they be waited for. -# If they invoke anything in threading.py that calls current_thread(), they -# leave an entry in the _active dict forever after. +# These should be added to `_active` and removed automatically +# when they die, although they can't be waited for. # Their purpose is to return *something* from current_thread(). # They are marked as daemon threads so we won't wait for them # when we exit (conform previous semantics). @@ -1495,6 +1528,7 @@ class _DummyThread(Thread): self._set_native_id() with _active_limbo_lock: _active[self._ident] = self + _DeleteDummyThreadOnDel(self) def _stop(self): pass @@ -1676,14 +1710,6 @@ def main_thread(): # XXX Figure this out for subinterpreters. (See gh-75698.) return _main_thread -# get thread-local implementation, either from the thread -# module, or from the python fallback - -try: - from _thread import _local as local -except ImportError: - from _threading_local import local - def _after_fork(): """ diff --git a/Misc/NEWS.d/next/Library/2024-01-22-11-43-38.gh-issue-114423.6mMoPH.rst b/Misc/NEWS.d/next/Library/2024-01-22-11-43-38.gh-issue-114423.6mMoPH.rst new file mode 100644 index 0000000..7b77b73 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-01-22-11-43-38.gh-issue-114423.6mMoPH.rst @@ -0,0 +1 @@ +``_DummyThread`` entries in ``threading._active`` are now automatically removed when the related thread dies. -- cgit v0.12