Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.12] gh-104341: Add a Separate "Running" Lock for Each Thread (gh-104754) #104817

Closed
wants to merge 1 commit into from
Closed

[3.12] gh-104341: Add a Separate "Running" Lock for Each Thread (gh-104754) #104817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions Lib/test/test_threading.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ def f():
rc, out, err = assert_python_ok("-c", code)
self.assertEqual(err, b"")

def test_tstate_lock(self):
def test_running_lock(self):
# Test an implementation detail of Thread objects.
started = _thread.allocate_lock()
finish = _thread.allocate_lock()
Expand All @@ -757,29 +757,29 @@ def f():
started.release()
finish.acquire()
time.sleep(0.01)
# The tstate lock is None until the thread is started
# The running lock is None until the thread is started
t = threading.Thread(target=f)
self.assertIs(t._tstate_lock, None)
self.assertIs(t._running_lock, None)
t.start()
started.acquire()
self.assertTrue(t.is_alive())
# The tstate lock can't be acquired when the thread is running
# The running lock can't be acquired when the thread is running
# (or suspended).
tstate_lock = t._tstate_lock
self.assertFalse(tstate_lock.acquire(timeout=0), False)
running_lock = t._running_lock
self.assertFalse(running_lock.acquire(timeout=0), False)
finish.release()
# When the thread ends, the state_lock can be successfully
# acquired.
self.assertTrue(tstate_lock.acquire(timeout=support.SHORT_TIMEOUT), False)
# But is_alive() is still True: we hold _tstate_lock now, which
# prevents is_alive() from knowing the thread's end-of-life C code
self.assertTrue(running_lock.acquire(timeout=support.SHORT_TIMEOUT), False)
# But is_alive() is still True: we hold _running_lock now, which
# prevents is_alive() from knowing the thread's Python code
# is done.
self.assertTrue(t.is_alive())
# Let is_alive() find out the C code is done.
tstate_lock.release()
running_lock.release()
self.assertFalse(t.is_alive())
# And verify the thread disposed of _tstate_lock.
self.assertIsNone(t._tstate_lock)
# And verify the thread disposed of _running_lock.
self.assertIsNone(t._running_lock)
t.join()

def test_repr_stopped(self):
Expand Down
70 changes: 41 additions & 29 deletions Lib/threading.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,7 @@ class is implemented.
self._ident = None
if _HAVE_THREAD_NATIVE_ID:
self._native_id = None
self._running_lock = None
self._tstate_lock = None
self._started = Event()
self._is_stopped = False
Expand All @@ -926,13 +927,17 @@ def _reset_internal_locks(self, is_alive):
# bpo-42350: If the fork happens when the thread is already stopped
# (ex: after threading._shutdown() has been called), _tstate_lock
# is None. Do nothing in this case.
if self._running_lock is not None:
self._running_lock._at_fork_reinit()
self._running_lock.acquire()
if self._tstate_lock is not None:
self._tstate_lock._at_fork_reinit()
self._tstate_lock.acquire()
else:
# The thread isn't alive after fork: it doesn't have a tstate
# anymore.
self._is_stopped = True
self._running_lock = None
self._tstate_lock = None

def __repr__(self):
Expand Down Expand Up @@ -1019,6 +1024,14 @@ def _set_ident(self):
def _set_native_id(self):
self._native_id = get_native_id()

def _set_running_lock(self):
"""
Set a lock object which will be released by the interpreter when
the target func has finished running.
"""
self._running_lock = _allocate_lock()
self._running_lock.acquire()

def _set_tstate_lock(self):
"""
Set a lock object which will be released by the interpreter when
Expand All @@ -1035,6 +1048,7 @@ def _set_tstate_lock(self):
def _bootstrap_inner(self):
try:
self._set_ident()
self._set_running_lock()
self._set_tstate_lock()
if _HAVE_THREAD_NATIVE_ID:
self._set_native_id()
Expand All @@ -1054,29 +1068,29 @@ def _bootstrap_inner(self):
self._invoke_excepthook(self)
finally:
self._delete()
self._running_lock.release()

def _stop(self):
# After calling ._stop(), .is_alive() returns False and .join() returns
# immediately. ._tstate_lock must be released before calling ._stop().
# immediately. ._running_lock must be released before calling ._stop().
#
# Normal case: C code at the end of the thread's life
# (release_sentinel in _threadmodule.c) releases ._tstate_lock, and
# that's detected by our ._wait_for_tstate_lock(), called by .join()
# Normal case: ._bootstrap_inner() releases ._running_lock, and
# that's detected by our ._wait_for_running_lock(), called by .join()
# and .is_alive(). Any number of threads _may_ call ._stop()
# simultaneously (for example, if multiple threads are blocked in
# .join() calls), and they're not serialized. That's harmless -
# they'll just make redundant rebindings of ._is_stopped and
# ._tstate_lock. Obscure: we rebind ._tstate_lock last so that the
# "assert self._is_stopped" in ._wait_for_tstate_lock() always works
# (the assert is executed only if ._tstate_lock is None).
# ._running_lock. Obscure: we rebind ._running_lock last so that the
# "assert self._is_stopped" in ._wait_for_running_lock() always works
# (the assert is executed only if ._running_lock is None).
#
# Special case: _main_thread releases ._tstate_lock via this
# Special case: _main_thread releases ._running_lock via this
# module's _shutdown() function.
lock = self._tstate_lock
lock = self._running_lock
if lock is not None:
assert not lock.locked()
self._is_stopped = True
self._tstate_lock = None
self._running_lock = None
if not self.daemon:
with _shutdown_locks_lock:
# Remove our lock and other released locks from _shutdown_locks
Expand Down Expand Up @@ -1123,20 +1137,17 @@ def join(self, timeout=None):
raise RuntimeError("cannot join current thread")

if timeout is None:
self._wait_for_tstate_lock()
self._wait_for_running_lock()
else:
# the behavior of a negative timeout isn't documented, but
# historically .join(timeout=x) for x<0 has acted as if timeout=0
self._wait_for_tstate_lock(timeout=max(timeout, 0))

def _wait_for_tstate_lock(self, block=True, timeout=-1):
# Issue #18808: wait for the thread state to be gone.
# At the end of the thread's life, after all knowledge of the thread
# is removed from C data structures, C code releases our _tstate_lock.
# This method passes its arguments to _tstate_lock.acquire().
# If the lock is acquired, the C code is done, and self._stop() is
# called. That sets ._is_stopped to True, and ._tstate_lock to None.
lock = self._tstate_lock
self._wait_for_running_lock(timeout=max(timeout, 0))

def _wait_for_running_lock(self, block=True, timeout=-1):
# This method passes its arguments to _running_lock.acquire().
# If the lock is acquired, the python code is done, and self._stop() is
# called. That sets ._is_stopped to True, and ._running_lock to None.
lock = self._running_lock
if lock is None:
# already determined that the C code is done
assert self._is_stopped
Expand Down Expand Up @@ -1207,7 +1218,7 @@ def is_alive(self):
assert self._initialized, "Thread.__init__() not called"
if self._is_stopped or not self._started.is_set():
return False
self._wait_for_tstate_lock(False)
self._wait_for_running_lock(False)
return not self._is_stopped

@property
Expand Down Expand Up @@ -1417,7 +1428,7 @@ class _MainThread(Thread):

def __init__(self):
Thread.__init__(self, name="MainThread", daemon=False)
self._set_tstate_lock()
self._set_running_lock()
self._started.set()
self._set_ident()
if _HAVE_THREAD_NATIVE_ID:
Expand Down Expand Up @@ -1558,7 +1569,7 @@ def _shutdown():
# dubious, but some code does it. We can't wait for C code to release
# the main thread's tstate_lock - that won't happen until the interpreter
# is nearly dead. So we release it here. Note that just calling _stop()
# isn't enough: other threads may already be waiting on _tstate_lock.
# isn't enough: other threads may already be waiting on _running_lock.
if _main_thread._is_stopped:
# _shutdown() was already called
return
Expand All @@ -1573,12 +1584,13 @@ def _shutdown():

# Main thread
if _main_thread.ident == get_ident():
tlock = _main_thread._tstate_lock
# The main thread isn't finished yet, so its thread state lock can't
assert _main_thread._tstate_lock is None
running_lock = _main_thread._running_lock
# The main thread isn't finished yet, so its running lock can't
# have been released.
assert tlock is not None
assert tlock.locked()
tlock.release()
assert running_lock is not None
assert running_lock.locked()
running_lock.release()
_main_thread._stop()
else:
# bpo-1596321: _shutdown() must be called in the main thread.
Expand Down