https://github.com/python/cpython/commit/dab59fa56054d6c0f75ae7013337f7baaa2... commit: dab59fa56054d6c0f75ae7013337f7baaa248076 branch: 2.7 author: Victor Stinner <vstinner@redhat.com> committer: GitHub <noreply@github.com> date: 2018-11-30T13:02:41+01:00 summary: 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) files: M Lib/test/test_socket.py diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 14607115928b..988e12a813d5 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -21,6 +21,9 @@ _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 @@ def _testSend(self): class NonBlockingTCPTests(ThreadedTCPSocketTest): def __init__(self, methodName='runTest'): + self.event = threading.Event() ThreadedTCPSocketTest.__init__(self, methodName=methodName) def testSetBlocking(self): @@ -982,21 +986,27 @@ def testSetBlocking_overflow(self): 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 @@ 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 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):