From 0c857865e4f255f99d58678f878e09c11da89892 Mon Sep 17 00:00:00 2001 From: Jacob Bower <1978924+jbower-fb@users.noreply.github.com> Date: Thu, 23 Feb 2023 14:57:06 -0800 Subject: [PATCH] Fix deadlock on shutdown if test_current_{exception,frames} fails (#102019) * 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 * Add a news entry * Fix whitespace --------- Co-authored-by: Brett Simmers Co-authored-by: Oleg Iarygin --- Lib/test/test_sys.py | 148 +++++++++--------- ...-02-18-10-51-02.gh-issue-102019.0797SJ.rst | 2 + 2 files changed, 77 insertions(+), 73 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2023-02-18-10-51-02.gh-issue-102019.0797SJ.rst diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 58aa9d10210edf..b839985def9a12 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -445,46 +445,47 @@ 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_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()") + 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") - # 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()"]) + self.assertEqual(sourceline, "g456()") - # Reap the spawned thread. - leave_g.set() - t.join() + # 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()") + 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") - # 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(")) + self.assertEqual(sourceline, "g456()") - # Reap the spawned thread. - leave_g.set() - t.join() + # 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) 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.