From 7b9c592e0a7121a9c1dc815e78671e42a4d23188 Mon Sep 17 00:00:00 2001 From: jbower <1978924+jbower-fb@users.noreply.github.com> Date: Wed, 18 Jan 2023 22:47:05 -0800 Subject: [PATCH 1/3] Don't deadlock on shutdown if test_current_{exception,frames} fails These tests spawn a thread that waits on a threading.Event. If the test fails any of its assertions, the Event won't be signaled and the thread will wait indefinitely, causing a deadlock when threading._shutdown() tries to join all outstanding threads. Co-authored-by: Brett Simmers --- Lib/test/test_sys.py | 158 ++++++++++++++++++++++--------------------- 1 file changed, 80 insertions(+), 78 deletions(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 58aa9d10210edf..480ff48af923ff 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -444,47 +444,48 @@ def g456(): t = threading.Thread(target=f123) t.start() entered_g.wait() - - # At this point, t has finished its entered_g.set(), although it's - # impossible to guess whether it's still on that line or has moved on - # to its leave_g.wait(). - self.assertEqual(len(thread_info), 1) - thread_id = thread_info[0] - - d = sys._current_frames() - for tid in d: - self.assertIsInstance(tid, int) - self.assertGreater(tid, 0) - - main_id = threading.get_ident() - self.assertIn(main_id, d) - self.assertIn(thread_id, d) - - # Verify that the captured main-thread frame is _this_ frame. - frame = d.pop(main_id) - self.assertTrue(frame is sys._getframe()) - - # Verify that the captured thread frame is blocked in g456, called - # from f123. This is a little tricky, since various bits of - # threading.py are also in the thread's call stack. - frame = d.pop(thread_id) - stack = traceback.extract_stack(frame) - for i, (filename, lineno, funcname, sourceline) in enumerate(stack): - if funcname == "f123": - break - else: - self.fail("didn't find f123() on thread's call stack") - - self.assertEqual(sourceline, "g456()") - - # And the next record must be for g456(). - filename, lineno, funcname, sourceline = stack[i+1] - self.assertEqual(funcname, "g456") - self.assertIn(sourceline, ["leave_g.wait()", "entered_g.set()"]) - - # Reap the spawned thread. - leave_g.set() - t.join() + + try: + # At this point, t has finished its entered_g.set(), although it's + # impossible to guess whether it's still on that line or has moved on + # to its leave_g.wait(). + self.assertEqual(len(thread_info), 1) + thread_id = thread_info[0] + + d = sys._current_frames() + for tid in d: + self.assertIsInstance(tid, int) + self.assertGreater(tid, 0) + + main_id = threading.get_ident() + self.assertIn(main_id, d) + self.assertIn(thread_id, d) + + # Verify that the captured main-thread frame is _this_ frame. + frame = d.pop(main_id) + self.assertTrue(frame is sys._getframe()) + + # Verify that the captured thread frame is blocked in g456, called + # from f123. This is a little tricky, since various bits of + # threading.py are also in the thread's call stack. + frame = d.pop(thread_id) + stack = traceback.extract_stack(frame) + for i, (filename, lineno, funcname, sourceline) in enumerate(stack): + if funcname == "f123": + break + else: + self.fail("didn't find f123() on thread's call stack") + + self.assertEqual(sourceline, "g456()") + + # And the next record must be for g456(). + filename, lineno, funcname, sourceline = stack[i+1] + self.assertEqual(funcname, "g456") + self.assertIn(sourceline, ["leave_g.wait()", "entered_g.set()"]) + finally: + # Reap the spawned thread. + leave_g.set() + t.join() @threading_helper.reap_threads @threading_helper.requires_working_threading() @@ -516,43 +517,44 @@ def g456(): t.start() entered_g.wait() - # At this point, t has finished its entered_g.set(), although it's - # impossible to guess whether it's still on that line or has moved on - # to its leave_g.wait(). - self.assertEqual(len(thread_info), 1) - thread_id = thread_info[0] - - d = sys._current_exceptions() - for tid in d: - self.assertIsInstance(tid, int) - self.assertGreater(tid, 0) - - main_id = threading.get_ident() - self.assertIn(main_id, d) - self.assertIn(thread_id, d) - self.assertEqual((None, None, None), d.pop(main_id)) - - # Verify that the captured thread frame is blocked in g456, called - # from f123. This is a little tricky, since various bits of - # threading.py are also in the thread's call stack. - exc_type, exc_value, exc_tb = d.pop(thread_id) - stack = traceback.extract_stack(exc_tb.tb_frame) - for i, (filename, lineno, funcname, sourceline) in enumerate(stack): - if funcname == "f123": - break - else: - self.fail("didn't find f123() on thread's call stack") - - self.assertEqual(sourceline, "g456()") - - # And the next record must be for g456(). - filename, lineno, funcname, sourceline = stack[i+1] - self.assertEqual(funcname, "g456") - self.assertTrue(sourceline.startswith("if leave_g.wait(")) - - # Reap the spawned thread. - leave_g.set() - t.join() + try: + # At this point, t has finished its entered_g.set(), although it's + # impossible to guess whether it's still on that line or has moved on + # to its leave_g.wait(). + self.assertEqual(len(thread_info), 1) + thread_id = thread_info[0] + + d = sys._current_exceptions() + for tid in d: + self.assertIsInstance(tid, int) + self.assertGreater(tid, 0) + + main_id = threading.get_ident() + self.assertIn(main_id, d) + self.assertIn(thread_id, d) + self.assertEqual((None, None, None), d.pop(main_id)) + + # Verify that the captured thread frame is blocked in g456, called + # from f123. This is a little tricky, since various bits of + # threading.py are also in the thread's call stack. + exc_type, exc_value, exc_tb = d.pop(thread_id) + stack = traceback.extract_stack(exc_tb.tb_frame) + for i, (filename, lineno, funcname, sourceline) in enumerate(stack): + if funcname == "f123": + break + else: + self.fail("didn't find f123() on thread's call stack") + + self.assertEqual(sourceline, "g456()") + + # And the next record must be for g456(). + filename, lineno, funcname, sourceline = stack[i+1] + self.assertEqual(funcname, "g456") + self.assertTrue(sourceline.startswith("if leave_g.wait(")) + finally: + # Reap the spawned thread. + leave_g.set() + t.join() def test_attributes(self): self.assertIsInstance(sys.api_version, int) From 2e3b2e76788581713fb70670351a0e679b5563d2 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 18 Feb 2023 10:51:11 +0400 Subject: [PATCH 2/3] Add a news entry --- .../next/Tests/2023-02-18-10-51-02.gh-issue-102019.0797SJ.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Tests/2023-02-18-10-51-02.gh-issue-102019.0797SJ.rst diff --git a/Misc/NEWS.d/next/Tests/2023-02-18-10-51-02.gh-issue-102019.0797SJ.rst b/Misc/NEWS.d/next/Tests/2023-02-18-10-51-02.gh-issue-102019.0797SJ.rst new file mode 100644 index 00000000000000..63e36046d26dfe --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2023-02-18-10-51-02.gh-issue-102019.0797SJ.rst @@ -0,0 +1,2 @@ +Fix deadlock on shutdown if ``test_current_{exception,frames}`` fails. Patch +by Jacob Bower. From 9a19c1064f9d0526e28053fd5557eaef09616829 Mon Sep 17 00:00:00 2001 From: jbower <1978924+jbower-fb@users.noreply.github.com> Date: Thu, 23 Feb 2023 14:25:22 -0800 Subject: [PATCH 3/3] Fix whitespace --- Lib/test/test_sys.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 480ff48af923ff..b839985def9a12 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -444,27 +444,27 @@ def g456(): t = threading.Thread(target=f123) t.start() entered_g.wait() - + try: # At this point, t has finished its entered_g.set(), although it's # impossible to guess whether it's still on that line or has moved on # to its leave_g.wait(). self.assertEqual(len(thread_info), 1) thread_id = thread_info[0] - + d = sys._current_frames() for tid in d: self.assertIsInstance(tid, int) self.assertGreater(tid, 0) - + main_id = threading.get_ident() self.assertIn(main_id, d) self.assertIn(thread_id, d) - + # Verify that the captured main-thread frame is _this_ frame. frame = d.pop(main_id) self.assertTrue(frame is sys._getframe()) - + # Verify that the captured thread frame is blocked in g456, called # from f123. This is a little tricky, since various bits of # threading.py are also in the thread's call stack. @@ -475,9 +475,9 @@ def g456(): break else: self.fail("didn't find f123() on thread's call stack") - + self.assertEqual(sourceline, "g456()") - + # And the next record must be for g456(). filename, lineno, funcname, sourceline = stack[i+1] self.assertEqual(funcname, "g456") @@ -523,17 +523,17 @@ def g456(): # to its leave_g.wait(). self.assertEqual(len(thread_info), 1) thread_id = thread_info[0] - + d = sys._current_exceptions() for tid in d: self.assertIsInstance(tid, int) self.assertGreater(tid, 0) - + main_id = threading.get_ident() self.assertIn(main_id, d) self.assertIn(thread_id, d) self.assertEqual((None, None, None), d.pop(main_id)) - + # Verify that the captured thread frame is blocked in g456, called # from f123. This is a little tricky, since various bits of # threading.py are also in the thread's call stack. @@ -544,9 +544,9 @@ def g456(): break else: self.fail("didn't find f123() on thread's call stack") - + self.assertEqual(sourceline, "g456()") - + # And the next record must be for g456(). filename, lineno, funcname, sourceline = stack[i+1] self.assertEqual(funcname, "g456")