https://github.com/python/cpython/commit/af7e81f71858519de8d2bafcb38fce8cca8... commit: af7e81f71858519de8d2bafcb38fce8cca86aa0a branch: 3.6 author: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> committer: GitHub <noreply@github.com> date: 2018-11-30T03:51:42-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@redhat.com> files: M Lib/test/test_socket.py diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index fbbc9f9abfb0..6b7afba49d9e 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -32,6 +32,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 try: import _thread as thread @@ -3870,6 +3871,7 @@ def _testSend(self): class NonBlockingTCPTests(ThreadedTCPSocketTest): def __init__(self, methodName='runTest'): + self.event = threading.Event() ThreadedTCPSocketTest.__init__(self, methodName=methodName) def testSetBlocking(self): @@ -3944,22 +3946,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): @@ -3974,25 +3981,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) @unittest.skipUnless(thread, 'Threading required for this test.') class FileObjectClassTestCase(SocketConnectedTest):