From 1ca8ad63227735fa190d2fc83c4797eec8d12b13 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 13 Jun 2019 10:53:53 +0200 Subject: [PATCH 1/3] bpo-36402: Fix threading.Thread._stop() Remove the _tstate_lock frm _shutdown_locks, don't remove None. --- Lib/threading.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/threading.py b/Lib/threading.py index 67926403770e62..7c6d404bcd10f6 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -965,7 +965,7 @@ def _stop(self): self._tstate_lock = None if not self.daemon: with _shutdown_locks_lock: - _shutdown_locks.discard(self._tstate_lock) + _shutdown_locks.discard(lock) def _delete(self): "Remove current thread from the dict of currently running threads." From 0da77a8d4713ccd76fdde45ebc589c689c9740ac Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 13 Jun 2019 11:10:25 +0200 Subject: [PATCH 2/3] Add unit tests --- Lib/test/test_threading.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index ad90010b8a3821..6b89301ed807ae 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -738,6 +738,22 @@ def callback(): finally: sys.settrace(old_trace) + @cpython_only + def test_shutdown_locks(self): + # test a non-daemon threads + event = threading.Event() + thread = threading.Thread(target=event.wait) + + # start() must add lock to _shutdown_locks + thread.start() + tstate_lock = thread._tstate_lock + self.assertIn(tstate_lock, threading._shutdown_locks) + + # _stop() must remove tstate_lock from _shutdown_locks + event.set() + thread.join() + self.assertNotIn(tstate_lock, threading._shutdown_locks) + class ThreadJoinOnShutdown(BaseTestCase): From 575d25c9485fca4c2ec8d3cb51a14e2f35f45a5a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 13 Jun 2019 11:23:57 +0200 Subject: [PATCH 3/3] Test also daemon threads --- Lib/test/test_threading.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 6b89301ed807ae..0a0a62bdf9bfaf 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -740,19 +740,27 @@ def callback(): @cpython_only def test_shutdown_locks(self): - # test a non-daemon threads - event = threading.Event() - thread = threading.Thread(target=event.wait) + for daemon in (False, True): + with self.subTest(daemon=daemon): + event = threading.Event() + thread = threading.Thread(target=event.wait, daemon=daemon) - # start() must add lock to _shutdown_locks - thread.start() - tstate_lock = thread._tstate_lock - self.assertIn(tstate_lock, threading._shutdown_locks) + # 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) - # _stop() must remove tstate_lock from _shutdown_locks - event.set() - thread.join() - 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):