fix flaky test ClientCloseTest.testStopLifecycle() in jetty 9.4.x (#3990)

* fix flaky test ClientCloseTest.testStopLifecycle() in 9.4.x

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

* test code cleanups in ClientCloseTest

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java

@@ -49,6 +49,7 @@ import org.eclipse.jetty.websocket.api.util.WSURI;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
 import org.eclipse.jetty.websocket.common.CloseInfo;
 import org.eclipse.jetty.websocket.common.OpCode;
+import org.eclipse.jetty.websocket.server.NativeWebSocketServletContainerInitializer;
 import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
 import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
 import org.eclipse.jetty.websocket.tests.CloseTrackingEndpoint;
@@ -60,7 +61,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.time.Duration.ofSeconds;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
@@ -83,10 +83,7 @@ public class ClientCloseTest
         {
             // Send message from client to server
             final String echoMsg = "echo-test";
-            Future<Void> testFut = clientSocket.getRemote().sendStringByFuture(echoMsg);
+            clientSocket.getRemote().sendString(echoMsg);
-
-            // Wait for send future
-            testFut.get(5, SECONDS);
 
             // Verify received message
             String recvMsg = clientSocket.messageQueue.poll(5, SECONDS);
@@ -108,7 +105,6 @@ public class ClientCloseTest
     {
         client = new WebSocketClient();
         client.setMaxTextMessageBufferSize(1024);
-        client.getPolicy().setMaxTextMessageSize(1024);
         client.start();
     }
 
@@ -143,6 +139,7 @@ public class ClientCloseTest
         handlers.addHandler(context);
         handlers.addHandler(new DefaultHandler());
         server.setHandler(handlers);
+        NativeWebSocketServletContainerInitializer.configure(context, null);
 
         server.start();
     }
@@ -174,9 +171,8 @@ public class ClientCloseTest
         CloseTrackingEndpoint clientSocket = new CloseTrackingEndpoint();
         Future<Session> clientConnectFuture = client.connect(clientSocket, wsUri);
 
-        try (Session session = confirmConnection(clientSocket, clientConnectFuture))
-        {
         // client confirms connection via echo
+        confirmConnection(clientSocket, clientConnectFuture);
 
         // client sends close frame (code 1000, normal)
         final String origCloseReason = "send-more-frames";
@@ -193,8 +189,6 @@ public class ClientCloseTest
 
         // client close event on ws-endpoint
         clientSocket.assertReceivedCloseEvent(timeout, is(StatusCode.NORMAL), containsString(""));
-        }
-
         clientSessionTracker.assertClosedProperly(client);
     }
 
@@ -213,18 +207,15 @@ public class ClientCloseTest
         CloseTrackingEndpoint clientSocket = new CloseTrackingEndpoint();
         Future<Session> clientConnectFuture = client.connect(clientSocket, wsUri);
 
-        try (Session session = confirmConnection(clientSocket, clientConnectFuture))
-        {
         // client confirms connection via echo
+        confirmConnection(clientSocket, clientConnectFuture);
 
-            session.getRemote().sendString("too-large-message");
+        clientSocket.getSession().getRemote().sendString("too-large-message");
-
         clientSocket.assertReceivedCloseEvent(timeout, is(StatusCode.MESSAGE_TOO_LARGE), containsString("exceeds maximum size"));
 
         // client should have noticed the error
         assertThat("OnError Latch", clientSocket.errorLatch.await(2, SECONDS), is(true));
         assertThat("OnError", clientSocket.error.get(), instanceOf(MessageTooLargeException.class));
-        }
 
         // client triggers close event on client ws-endpoint
         clientSessionTracker.assertClosedProperly(client);
@@ -234,7 +225,7 @@ public class ClientCloseTest
     public void testRemoteDisconnect() throws Exception
     {
         // Set client timeout
-        final int clientTimeout = 1000;
+        final int clientTimeout = 3000;
         client.setMaxIdleTimeout(clientTimeout);
 
         ClientOpenSessionTracker clientSessionTracker = new ClientOpenSessionTracker(1);
@@ -245,19 +236,19 @@ public class ClientCloseTest
         CloseTrackingEndpoint clientSocket = new CloseTrackingEndpoint();
         Future<Session> clientConnectFuture = client.connect(clientSocket, wsUri);
 
-        try (Session ignored = confirmConnection(clientSocket, clientConnectFuture))
-        {
         // client confirms connection via echo
+        confirmConnection(clientSocket, clientConnectFuture);
 
         // client sends close frame (triggering server connection abort)
         final String origCloseReason = "abort";
         clientSocket.getSession().close(StatusCode.NORMAL, origCloseReason);
 
         // client reads -1 (EOF)
-            // client triggers close event on client ws-endpoint
+        // client triggers hard close event on client ws-endpoint
-            // assert - close code==1006 (abnormal) or code==1001 (shutdown)
+        // assert - close code==1006 (abnormal)
-            clientSocket.assertReceivedCloseEvent(clientTimeout * 2, anyOf(is(StatusCode.SHUTDOWN), is(StatusCode.ABNORMAL)));
+        clientSocket.assertReceivedCloseEvent(2000,
-        }
+            is(StatusCode.ABNORMAL),
+            containsString("Disconnected"));
 
         clientSessionTracker.assertClosedProperly(client);
     }
@@ -277,12 +268,11 @@ public class ClientCloseTest
         CloseTrackingEndpoint clientSocket = new CloseTrackingEndpoint();
         Future<Session> clientConnectFuture = client.connect(clientSocket, wsUri);
 
-        try (Session ignored = confirmConnection(clientSocket, clientConnectFuture))
-        {
         // client confirms connection via echo
+        confirmConnection(clientSocket, clientConnectFuture);
 
         // client sends close frame
-            final String origCloseReason = "sleep|5000";
+        final String origCloseReason = "sleep|2500";
         clientSocket.getSession().close(StatusCode.NORMAL, origCloseReason);
 
         // client close should occur
@@ -294,8 +284,6 @@ public class ClientCloseTest
         assertThat("OnError Latch", clientSocket.errorLatch.await(2, SECONDS), is(true));
         assertThat("OnError", clientSocket.error.get(), instanceOf(CloseException.class));
         assertThat("OnError.cause", clientSocket.error.get().getCause(), instanceOf(TimeoutException.class));
-        }
-
         clientSessionTracker.assertClosedProperly(client);
     }
 
@@ -303,7 +291,7 @@ public class ClientCloseTest
     public void testStopLifecycle() throws Exception
     {
         // Set client timeout
-        final int timeout = 1000;
+        final int timeout = 3000;
         client.setMaxIdleTimeout(timeout);
 
         int sessionCount = 3;
@@ -325,6 +313,16 @@ public class ClientCloseTest
             confirmConnection(clientSocket, clientConnectFuture);
         }
 
+        assertThat(serverEndpoints.size(), is(sessionCount));
+
+        try
+        {
+            // block all the server threads
+            for (int i = 0; i < sessionCount; i++)
+            {
+                clientSockets.get(i).getSession().getRemote().sendString("block");
+            }
+
             assertTimeoutPreemptively(ofSeconds(5), () ->
             {
                 // client lifecycle stop (the meat of this test)
@@ -334,12 +332,22 @@ public class ClientCloseTest
             // clients disconnect
             for (int i = 0; i < sessionCount; i++)
             {
-            clientSockets.get(i).assertReceivedCloseEvent(timeout, is(StatusCode.ABNORMAL), containsString("Disconnected"));
+                // The Disconnect callback is succeeded directly after sending the SHUTDOWN close frame so we get ABNORMAL close
+                clientSockets.get(i).assertReceivedCloseEvent(2000, is(StatusCode.ABNORMAL), containsString("Disconnected"));
             }
 
             // ensure all Sessions are gone. connections are gone. etc. (client and server)
             // ensure ConnectionListener onClose is called 3 times
             clientSessionTracker.assertClosedProperly(client);
+            assertThat(serverEndpoints.size(), is(sessionCount));
+        }
+        finally
+        {
+            for (int i = 0; i < sessionCount; i++)
+            {
+                serverEndpoints.get(i).block.countDown();
+            }
+        }
     }
 
     @Test