bpo-35998: Fix test_asyncio.test_start_tls_server_1() (GH-16815)

vstinner · web-flow · commit fab4ef2df085 · 2019-10-16T02:36:42.000+02:00
main() is now responsible to send the ANSWER, rather than
ServerProto. main() now waits until it got the HELLO before sending
the ANSWER over the new transport.

Previously, there was a race condition between main() replacing the
protocol and the protocol sending the ANSWER once it gets the HELLO.

TLSv1.3 was disabled for the test: reenable it.
diff --git a/Lib/test/test_asyncio/test_sslproto.py b/Lib/test/test_asyncio/test_sslproto.py
@@ -497,14 +497,6 @@ def test_start_tls_server_1(self):
 
         server_context = test_utils.simple_server_sslcontext()
         client_context = test_utils.simple_client_sslcontext()
-        if (sys.platform.startswith('freebsd')
-                or sys.platform.startswith('win')
-                or sys.platform.startswith('darwin')):
-            # bpo-35031: Some FreeBSD and Windows buildbots fail to run this test
-            # as the eof was not being received by the server if the payload
-            # size is not big enough. This behaviour only appears if the
-            # client is using TLS1.3.  Also seen on macOS.
-            client_context.options |= ssl.OP_NO_TLSv1_3
         answer = None
 
         def client(sock, addr):
@@ -521,9 +513,10 @@ def client(sock, addr):
             sock.close()
 
         class ServerProto(asyncio.Protocol):
-            def __init__(self, on_con, on_con_lost):
+            def __init__(self, on_con, on_con_lost, on_got_hello):
                 self.on_con = on_con
                 self.on_con_lost = on_con_lost
+                self.on_got_hello = on_got_hello
                 self.data = b''
                 self.transport = None
 
@@ -537,7 +530,7 @@ def replace_transport(self, tr):
             def data_received(self, data):
                 self.data += data
                 if len(self.data) >= len(HELLO_MSG):
-                    self.transport.write(ANSWER)
+                    self.on_got_hello.set_result(None)
 
             def connection_lost(self, exc):
                 self.transport = None
@@ -546,7 +539,7 @@ def connection_lost(self, exc):
                 else:
                     self.on_con_lost.set_exception(exc)
 
-        async def main(proto, on_con, on_con_lost):
+        async def main(proto, on_con, on_con_lost, on_got_hello):
             tr = await on_con
             tr.write(HELLO_MSG)
 
@@ -556,17 +549,20 @@ async def main(proto, on_con, on_con_lost):
                 tr, proto, server_context,
                 server_side=True,
                 ssl_handshake_timeout=self.TIMEOUT)
-
             proto.replace_transport(new_tr)
 
+            await on_got_hello
+            new_tr.write(ANSWER)
+
             await on_con_lost
             self.assertEqual(proto.data, HELLO_MSG)
             new_tr.close()
 
         async def run_main():
             on_con = self.loop.create_future()
             on_con_lost = self.loop.create_future()
-            proto = ServerProto(on_con, on_con_lost)
+            on_got_hello = self.loop.create_future()
+            proto = ServerProto(on_con, on_con_lost, on_got_hello)
 
             server = await self.loop.create_server(
                 lambda: proto, '127.0.0.1', 0)
@@ -575,7 +571,7 @@ async def run_main():
             with self.tcp_client(lambda sock: client(sock, addr),
                                  timeout=self.TIMEOUT):
                 await asyncio.wait_for(
-                    main(proto, on_con, on_con_lost),
+                    main(proto, on_con, on_con_lost, on_got_hello),
                     timeout=self.TIMEOUT)
 
             server.close()
diff --git a/Misc/NEWS.d/next/Tests/2019-10-16-01-36-15.bpo-35998.G305Bf.rst b/Misc/NEWS.d/next/Tests/2019-10-16-01-36-15.bpo-35998.G305Bf.rst
@@ -0,0 +1,5 @@
+Fix a race condition in test_asyncio.test_start_tls_server_1(). Previously,
+there was a race condition between the test main() function which replaces the
+protocol and the test ServerProto protocol which sends ANSWER once it gets
+HELLO. Now, only the test main() function is responsible to send data,
+ServerProto no longer sends data.
