bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791) (GH-10817)

testAccept() and testRecv() of test_socket.NonBlockingTCPTests have a
race condition: time.sleep() is used as a weak synchronization
primitive and the tests fail randomly on slow buildbots.

Use a reliable threading.Event to fix these tests.

Other changes:

* Replace send() with sendall()
* Add a timeout to select() in testAccept() and testRecv()
* Use addCleanup() to close sockets
* Use assertRaises()

(cherry picked from commit ebd5d6d6e6e4e751ba9c7534004aadfc27ba9265)

diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 14607115928b8c1058216998f6856fabefa1a843..988e12a813d5fb95a3e450403d7271785d4a712d 100644 (file)
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -21,6 +21,9 @@ except ImportError:
     _socket = None
 
 
+MAIN_TIMEOUT = 60.0
+
+
 def try_address(host, port=0, family=socket.AF_INET):
     """Try to bind a socket on the given host:port and return True
     if that has been possible."""
@@ -947,6 +950,7 @@ class BasicSocketPairTest(SocketPairTest):
 class NonBlockingTCPTests(ThreadedTCPSocketTest):
 
     def __init__(self, methodName='runTest'):
+        self.event = threading.Event()
         ThreadedTCPSocketTest.__init__(self, methodName=methodName)
 
     def testSetBlocking(self):
@@ -982,21 +986,27 @@ class NonBlockingTCPTests(ThreadedTCPSocketTest):
     def testAccept(self):
         # Testing non-blocking accept
         self.serv.setblocking(0)
-        try:
-            conn, addr = self.serv.accept()
-        except socket.error:
-            pass
-        else:
-            self.fail("Error trying to do non-blocking accept.")
-        read, write, err = select.select([self.serv], [], [])
-        if self.serv in read:
+
+        # connect() didn't start: non-blocking accept() fails
+        with self.assertRaises(socket.error):
             conn, addr = self.serv.accept()
-            conn.close()
-        else:
+
+        self.event.set()
+
+        read, write, err = select.select([self.serv], [], [], MAIN_TIMEOUT)
+        if self.serv not in read:
             self.fail("Error trying to do accept after select.")
 
+        # connect() completed: non-blocking accept() doesn't block
+        conn, addr = self.serv.accept()
+        self.addCleanup(conn.close)
+        self.assertIsNone(conn.gettimeout())
+
     def _testAccept(self):
-        time.sleep(0.1)
+        # don't connect before event is set to check
+        # that non-blocking accept() raises socket.error
+        self.event.wait()
+
         self.cli.connect((HOST, self.port))
 
     def testConnect(self):
@@ -1011,25 +1021,32 @@ class NonBlockingTCPTests(ThreadedTCPSocketTest):
     def testRecv(self):
         # Testing non-blocking recv
         conn, addr = self.serv.accept()
+        self.addCleanup(conn.close)
         conn.setblocking(0)
-        try:
-            msg = conn.recv(len(MSG))
-        except socket.error:
-            pass
-        else:
-            self.fail("Error trying to do non-blocking recv.")
-        read, write, err = select.select([conn], [], [])
-        if conn in read:
+
+        # the server didn't send data yet: non-blocking recv() fails
+        with self.assertRaises(socket.error):
             msg = conn.recv(len(MSG))
-            conn.close()
-            self.assertEqual(msg, MSG)
-        else:
+
+        self.event.set()
+
+        read, write, err = select.select([conn], [], [], MAIN_TIMEOUT)
+        if conn not in read:
             self.fail("Error during select call to non-blocking socket.")
 
+        # the server sent data yet: non-blocking recv() doesn't block
+        msg = conn.recv(len(MSG))
+        self.assertEqual(msg, MSG)
+
     def _testRecv(self):
         self.cli.connect((HOST, self.port))
-        time.sleep(0.1)
-        self.cli.send(MSG)
+
+        # don't send anything before event is set to check
+        # that non-blocking recv() raises socket.error
+        self.event.wait()
+
+        # send data: recv() will no longer block
+        self.cli.sendall(MSG)
 
 @unittest.skipUnless(thread, 'Threading required for this test.')
 class FileObjectClassTestCase(SocketConnectedTest):