diff options
author | Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> | 2019-06-13 05:22:20 -0700 |
---|---|---|
committer | Victor Stinner <vstinner@redhat.com> | 2019-06-13 14:22:20 +0200 |
commit | 6eb2878e42152e9c45d7ee5e6f889532d753e67c (patch) | |
tree | 5af19e9d78170a19b388f9e7c2c329f3820f4006 /Lib | |
parent | b4c8ef7c67712c6639ee896bf7cb8ca1c204946d (diff) | |
download | cpython-git-6eb2878e42152e9c45d7ee5e6f889532d753e67c.tar.gz |
bpo-36402: Fix threading._shutdown() race condition (GH-13948) (GH-14050) (GH-14054)
* bpo-36402: Fix threading._shutdown() race condition (GH-13948)
Fix a race condition at Python shutdown when waiting for threads.
Wait until the Python thread state of all non-daemon threads get
deleted (join all non-daemon threads), rather than just wait until
Python threads complete.
* Add threading._shutdown_locks: set of Thread._tstate_lock locks
of non-daemon threads used by _shutdown() to wait until all Python
thread states get deleted. See Thread._set_tstate_lock().
* Add also threading._shutdown_locks_lock to protect access to
threading._shutdown_locks.
* Add test_finalization_shutdown() test.
(cherry picked from commit 468e5fec8a2f534f1685d59da3ca4fad425c38dd)
* bpo-36402: Fix threading.Thread._stop() (GH-14047)
Remove the _tstate_lock from _shutdown_locks, don't remove None.
(cherry picked from commit 6f75c873752a16a7ad8f35855b1e29f59d048e84)
(cherry picked from commit e40a97a721d46307dfdc2b0322028ccded6eb571)
Co-authored-by: Victor Stinner <vstinner@redhat.com>
Diffstat (limited to 'Lib')
-rw-r--r-- | Lib/test/test_threading.py | 79 | ||||
-rw-r--r-- | Lib/threading.py | 49 |
2 files changed, 116 insertions, 12 deletions
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index aa810bda1c..36d9fbb162 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -578,6 +578,41 @@ class ThreadTests(BaseTestCase): self.assertEqual(data.splitlines(), ["GC: True True True"] * 2) + def test_finalization_shutdown(self): + # bpo-36402: Py_Finalize() calls threading._shutdown() which must wait + # until Python thread states of all non-daemon threads get deleted. + # + # Test similar to SubinterpThreadingTests.test_threads_join_2(), but + # test the finalization of the main interpreter. + code = """if 1: + import os + import threading + import time + import random + + def random_sleep(): + seconds = random.random() * 0.010 + time.sleep(seconds) + + class Sleeper: + def __del__(self): + random_sleep() + + tls = threading.local() + + def f(): + # Sleep a bit so that the thread is still running when + # Py_Finalize() is called. + random_sleep() + tls.x = Sleeper() + random_sleep() + + threading.Thread(target=f).start() + random_sleep() + """ + rc, out, err = assert_python_ok("-c", code) + self.assertEqual(err, b"") + def test_tstate_lock(self): # Test an implementation detail of Thread objects. started = _thread.allocate_lock() @@ -698,6 +733,30 @@ class ThreadTests(BaseTestCase): finally: sys.settrace(old_trace) + @cpython_only + def test_shutdown_locks(self): + for daemon in (False, True): + with self.subTest(daemon=daemon): + event = threading.Event() + thread = threading.Thread(target=event.wait, daemon=daemon) + + # Thread.start() must add lock to _shutdown_locks, + # but only for non-daemon thread + thread.start() + tstate_lock = thread._tstate_lock + if not daemon: + self.assertIn(tstate_lock, threading._shutdown_locks) + else: + self.assertNotIn(tstate_lock, threading._shutdown_locks) + + # unblock the thread and join it + event.set() + thread.join() + + # Thread._stop() must remove tstate_lock from _shutdown_locks. + # Daemon threads must never add it to _shutdown_locks. + self.assertNotIn(tstate_lock, threading._shutdown_locks) + class ThreadJoinOnShutdown(BaseTestCase): @@ -875,15 +934,22 @@ class SubinterpThreadingTests(BaseTestCase): self.addCleanup(os.close, w) code = r"""if 1: import os + import random import threading import time + def random_sleep(): + seconds = random.random() * 0.010 + time.sleep(seconds) + def f(): # Sleep a bit so that the thread is still running when # Py_EndInterpreter is called. - time.sleep(0.05) + random_sleep() os.write(%d, b"x") + threading.Thread(target=f).start() + random_sleep() """ % (w,) ret = test.support.run_in_subinterp(code) self.assertEqual(ret, 0) @@ -900,22 +966,29 @@ class SubinterpThreadingTests(BaseTestCase): self.addCleanup(os.close, w) code = r"""if 1: import os + import random import threading import time + def random_sleep(): + seconds = random.random() * 0.010 + time.sleep(seconds) + class Sleeper: def __del__(self): - time.sleep(0.05) + random_sleep() tls = threading.local() def f(): # Sleep a bit so that the thread is still running when # Py_EndInterpreter is called. - time.sleep(0.05) + random_sleep() tls.x = Sleeper() os.write(%d, b"x") + threading.Thread(target=f).start() + random_sleep() """ % (w,) ret = test.support.run_in_subinterp(code) self.assertEqual(ret, 0) diff --git a/Lib/threading.py b/Lib/threading.py index 318b330112..0fb3bdd55c 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -733,6 +733,11 @@ _active_limbo_lock = _allocate_lock() _active = {} # maps thread id to Thread object _limbo = {} _dangling = WeakSet() +# Set of Thread._tstate_lock locks of non-daemon threads used by _shutdown() +# to wait until all Python thread states get deleted: +# see Thread._set_tstate_lock(). +_shutdown_locks_lock = _allocate_lock() +_shutdown_locks = set() # Main class for threads @@ -899,6 +904,10 @@ class Thread: self._tstate_lock = _set_sentinel() self._tstate_lock.acquire() + if not self.daemon: + with _shutdown_locks_lock: + _shutdown_locks.add(self._tstate_lock) + def _bootstrap_inner(self): try: self._set_ident() @@ -987,6 +996,9 @@ class Thread: assert not lock.locked() self._is_stopped = True self._tstate_lock = None + if not self.daemon: + with _shutdown_locks_lock: + _shutdown_locks.discard(lock) def _delete(self): "Remove current thread from the dict of currently running threads." @@ -1261,6 +1273,9 @@ from _thread import stack_size _main_thread = _MainThread() def _shutdown(): + """ + Wait until the Python thread state of all non-daemon threads get deleted. + """ # Obscure: other threads may be waiting to join _main_thread. That's # 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 @@ -1269,6 +1284,8 @@ def _shutdown(): if _main_thread._is_stopped: # _shutdown() was already called return + + # Main thread tlock = _main_thread._tstate_lock # The main thread isn't finished yet, so its thread state lock can't have # been released. @@ -1276,16 +1293,24 @@ def _shutdown(): assert tlock.locked() tlock.release() _main_thread._stop() - t = _pickSomeNonDaemonThread() - while t: - t.join() - t = _pickSomeNonDaemonThread() -def _pickSomeNonDaemonThread(): - for t in enumerate(): - if not t.daemon and t.is_alive(): - return t - return None + # Join all non-deamon threads + while True: + with _shutdown_locks_lock: + locks = list(_shutdown_locks) + _shutdown_locks.clear() + + if not locks: + break + + for lock in locks: + # mimick Thread.join() + lock.acquire() + lock.release() + + # new threads can be spawned while we were waiting for the other + # threads to complete + def main_thread(): """Return the main thread object. @@ -1311,12 +1336,18 @@ def _after_fork(): # Reset _active_limbo_lock, in case we forked while the lock was held # by another (non-forked) thread. http://bugs.python.org/issue874900 global _active_limbo_lock, _main_thread + global _shutdown_locks_lock, _shutdown_locks _active_limbo_lock = _allocate_lock() # fork() only copied the current thread; clear references to others. new_active = {} current = current_thread() _main_thread = current + + # reset _shutdown() locks: threads re-register their _tstate_lock below + _shutdown_locks_lock = _allocate_lock() + _shutdown_locks = set() + with _active_limbo_lock: # Dangling thread instances must still have their locks reset, # because someone may join() them. |