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 <bsimmers@meta.com> * Add a news entry * Fix whitespace --------- Co-authored-by: Brett Simmers <bsimmers@meta.com> Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>

Author: 3 people

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)

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.