Skip to content

Commit

Permalink
Fix deadlock on shutdown if test_current_{exception,frames} fails (py…
Browse files Browse the repository at this point in the history
…thon#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>
  • Loading branch information
3 people committed Feb 23, 2023
1 parent ccd98a3 commit 0c85786
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 73 deletions.
148 changes: 75 additions & 73 deletions Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix deadlock on shutdown if ``test_current_{exception,frames}`` fails. Patch
by Jacob Bower.

0 comments on commit 0c85786

Please sign in to comment.