[Python-checkins] bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791)

[Miss Islington (bot)]

https://github.com/python/cpython/commit/365f21c2d3756a6768c5b0d479aaf5c2a568b80b
commit: 365f21c2d3756a6768c5b0d479aaf5c2a568b80b
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2018-11-30T03:48:17-08:00
summary:

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


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()
* Expect specific BlockingIOError rather than generic OSError
* Add a timeout to select() in testAccept() and testRecv()
* Use addCleanup() to close sockets
* Use assertRaises()
(cherry picked from commit ebd5d6d6e6e4e751ba9c7534004aadfc27ba9265)

Co-authored-by: Victor Stinner <vstinner at redhat.com>

files:
M Lib/test/test_socket.py

diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 5f9891de4936..db2160eaf583 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -35,6 +35,7 @@
 
 HOST = support.HOST
 MSG = 'Michael Gilfix was here\u1234\r\n'.encode('utf-8') ## test unicode string and carriage return
+MAIN_TIMEOUT = 60.0
 
 VSOCKPORT = 1234
 
@@ -4174,6 +4175,7 @@ def _testSend(self):
 class NonBlockingTCPTests(ThreadedTCPSocketTest):
 
     def __init__(self, methodName='runTest'):
+        self.event = threading.Event()
         ThreadedTCPSocketTest.__init__(self, methodName=methodName)
 
     def testSetBlocking(self):
@@ -4286,22 +4288,27 @@ def _testInheritFlags(self):
     def testAccept(self):
         # Testing non-blocking accept
         self.serv.setblocking(0)
-        try:
-            conn, addr = self.serv.accept()
-        except OSError:
-            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(BlockingIOError):
             conn, addr = self.serv.accept()
-            self.assertIsNone(conn.gettimeout())
-            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 BlockingIOError
+        self.event.wait()
+
         self.cli.connect((HOST, self.port))
 
     def testConnect(self):
@@ -4316,25 +4323,32 @@ def _testConnect(self):
     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 OSError:
-            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(BlockingIOError):
             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 BlockingIOError
+        self.event.wait()
+
+        # send data: recv() will no longer block
+        self.cli.sendall(MSG)
 
 
 class FileObjectClassTestCase(SocketConnectedTest):