Skip to content

gh-110167: Fix test_socket deadlock in doCleanups() #110416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vstinner merged 1 commit into from
Oct 5, 2023
Merged

gh-110167: Fix test_socket deadlock in doCleanups() #110416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
gh-110167: Fix test_socket deadlock in doCleanups() 
Fix a deadlock in test_socket when server fails with a timeout but
the client is still running in its thread. Don't hold a lock to call
cleanup functions in doCleanups(). One of the cleanup function waits
until the client completes, whereas the client could deadlock if it
called addCleanup() in such situation.

doCleanups() is called when the server completed, but the client can
still be running in its thread especially if the server failed with a
timeout. Don't put a lock on doCleanups() to prevent deadlock between
addCleanup() called in the client and doCleanups() waiting for
self.done.wait of ThreadableTest._setUp().
  • Loading branch information
@vstinner
vstinner committed Oct 5, 2023
commit 98ac8981de7b1818f0d1d7042878399a7d966754
12 changes: 7 additions & 5 deletions Lib/test/test_socket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,13 @@ def setUp(self):
class ThreadSafeCleanupTestCase:
"""Subclass of unittest.TestCase with thread-safe cleanup methods.

This subclass protects the addCleanup() and doCleanups() methods
with a recursive lock.
This subclass protects the addCleanup() method with a recursive lock.

doCleanups() is called when the server completed, but the client can still
be running in its thread especially if the server failed with a timeout.
Don't put a lock on doCleanups() to prevent deadlock between addCleanup()
called in the client and doCleanups() waiting for self.done.wait of
ThreadableTest._setUp() (gh-110167)
"""

def __init__(self, *args, **kwargs):
Expand All @@ -230,9 +235,6 @@ def addCleanup(self, *args, **kwargs):
with self._cleanup_lock:
return super().addCleanup(*args, **kwargs)

def doCleanups(self, *args, **kwargs):
with self._cleanup_lock:
return super().doCleanups(*args, **kwargs)

class SocketCANTest(unittest.TestCase):

Expand Down
5 changes: 5 additions & 0 deletions Misc/NEWS.d/next/Tests/2023-10-05-19-33-49.gh-issue-110167.mIdj3v.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix a deadlock in test_socket when server fails with a timeout but the
client is still running in its thread. Don't hold a lock to call cleanup
functions in doCleanups(). One of the cleanup function waits until the
client completes, whereas the client could deadlock if it called
addCleanup() in such situation. Patch by Victor Stinner.