[3.11] gh-110167: Fix test_socket deadlock in doCleanups() (GH-110416) (#110424)

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

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().
(cherry picked from commit 318f5df27109ff8d2519edefa771920a0ec62b92)

Co-authored-by: Victor Stinner <vstinner@python.org>

diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 190efb416ba39a5e79d690929306d0bd64ca2dce..bc93d9988eaf03cd1b26749f8d1fa37a8a305db5 100644 (file)
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -204,8 +204,13 @@ class SocketUDPLITETest(SocketUDPTest):
 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):
@@ -216,9 +221,6 @@ class ThreadSafeCleanupTestCase:
         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):
 

diff --git a/Misc/NEWS.d/next/Tests/2023-10-05-19-33-49.gh-issue-110167.mIdj3v.rst b/Misc/NEWS.d/next/Tests/2023-10-05-19-33-49.gh-issue-110167.mIdj3v.rst
new file mode 100644 (file)
index 0000000..d0cbbf9
--- /dev/null
+++ b/Misc/NEWS.d/next/Tests/2023-10-05-19-33-49.gh-issue-110167.mIdj3v.rst
@@ -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.