#10226 assert using awaitility and fix heap dump cleanup when a leak is detected

Signed-off-by: Ludovic Orban <lorban@bitronix.be>

jetty-core/jetty-fcgi/jetty-fcgi-server/pom.xml

@@ -22,6 +22,8 @@
           <argLine>
             @{argLine} ${jetty.surefire.argLine}
             --add-reads org.eclipse.jetty.fcgi.server=org.eclipse.jetty.logging
+            --add-reads org.eclipse.jetty.fcgi.server=java.management
+            --add-reads org.eclipse.jetty.fcgi.server=jdk.management
           </argLine>
         </configuration>
       </plugin>
@@ -61,5 +63,10 @@
       <artifactId>jetty-unixdomain-server</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.awaitility</groupId>
+      <artifactId>awaitility</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>

jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java

@@ -13,6 +13,15 @@
 
 package org.eclipse.jetty.fcgi.server;
 
+import java.lang.management.ManagementFactory;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
+import javax.management.MBeanServer;
+
+import org.awaitility.Awaitility;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.HttpClientTransport;
 import org.eclipse.jetty.fcgi.client.transport.HttpClientTransportOverFCGI;
@@ -28,8 +37,9 @@ import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.TestInfo;
 
-import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.fail;
 
 public abstract class AbstractHttpClientServerTest
 {
@@ -67,14 +77,36 @@ public abstract class AbstractHttpClientServerTest
     }
 
     @AfterEach
-    public void dispose()
+    public void dispose(TestInfo testInfo) throws Exception
     {
         try
         {
             if (serverBufferPool != null)
-                assertThat("Server Leaks: " + serverBufferPool.getLeaks(), serverBufferPool.getLeaks().size(), Matchers.is(0));
+            {
+                try
+                {
+                    Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> serverBufferPool.getLeaks().size(), Matchers.is(0));
+                }
+                catch (Exception e)
+                {
+                    String className = testInfo.getTestClass().orElseThrow().getName();
+                    dumpHeap("server-" + className);
+                    fail(e.getMessage() + "\n---\nServer Leaks: " + serverBufferPool.dumpLeaks() + "---\n");
+                }
+            }
             if (clientBufferPool != null)
-                assertThat("Client Leaks: " + clientBufferPool.getLeaks(), clientBufferPool.getLeaks().size(), Matchers.is(0));
+            {
+                try
+                {
+                    Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> clientBufferPool.getLeaks().size(), Matchers.is(0));
+                }
+                catch (Exception e)
+                {
+                    String className = testInfo.getTestClass().orElseThrow().getName();
+                    dumpHeap("client-" + className);
+                    fail(e.getMessage() + "\n---\nClient Leaks: " + clientBufferPool.dumpLeaks() + "---\n");
+                }
+            }
         }
         finally
         {
@@ -82,4 +114,26 @@ public abstract class AbstractHttpClientServerTest
             LifeCycle.stop(server);
         }
     }
+
+    private static void dumpHeap(String testMethodName) throws Exception
+    {
+        Path targetDir = Path.of("target/leaks");
+        if (Files.exists(targetDir))
+        {
+            try (Stream<Path> stream = Files.walk(targetDir))
+            {
+                stream.sorted(Comparator.reverseOrder())
+                    .map(Path::toFile)
+                    .forEach(java.io.File::delete);
+            }
+        }
+        Files.createDirectories(targetDir);
+        String dumpName = targetDir.resolve(testMethodName + ".hprof").toString();
+
+        MBeanServer server = ManagementFactory.getPlatformMBeanServer();
+        Class<?> mxBeanClass = Class.forName("com.sun.management.HotSpotDiagnosticMXBean");
+        Object mxBean = ManagementFactory.newPlatformMXBeanProxy(
+            server, "com.sun.management:type=HotSpotDiagnostic", mxBeanClass);
+        mxBeanClass.getMethod("dumpHeap", String.class, boolean.class).invoke(mxBean, dumpName, true);
+    }
 }

jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java

@@ -240,6 +240,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
                 String paramValue2 = fields.getValue(paramName2);
                 assertEquals("", paramValue2);
                 Content.Sink.write(response, true, UTF_8.encode("empty"));
+                callback.succeeded();
                 return true;
             }
         });
@@ -274,6 +275,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
                 }
                 String paramValue2 = fields.getValue(paramName2);
                 Content.Sink.write(response, true, UTF_8.encode(paramValue2));
+                callback.succeeded();
                 return true;
             }
         });
@@ -650,6 +652,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
     @Test
     public void testLongPollIsAbortedWhenClientIsStopped() throws Exception
     {
+        AtomicReference<Callback> callbackRef = new AtomicReference<>();
         CountDownLatch latch = new CountDownLatch(1);
         start(new Handler.Abstract()
         {
@@ -657,26 +660,38 @@ public class HttpClientTest extends AbstractHttpClientServerTest
             public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jetty.server.Response response, Callback callback)
             {
                 latch.countDown();
-                // Do not complete the callback.
+                // Do not complete the callback, but store it aside for
+                // releasing the buffer later on.
+                callbackRef.set(callback);
                 return true;
             }
         });
 
-        CountDownLatch completeLatch = new CountDownLatch(1);
+        try
-        client.newRequest("localhost", connector.getLocalPort())
+        {
-            .scheme(scheme)
+            CountDownLatch completeLatch = new CountDownLatch(1);
-            .send(result ->
+            client.newRequest("localhost", connector.getLocalPort())
-            {
+                .scheme(scheme)
-                if (result.isFailed())
+                .send(result ->
-                    completeLatch.countDown();
+                {
-            });
+                    if (result.isFailed())
+                        completeLatch.countDown();
+                });
 
-        assertTrue(latch.await(5, TimeUnit.SECONDS));
+            assertTrue(latch.await(5, TimeUnit.SECONDS));
 
-        // Stop the client, the complete listener must be invoked.
+            // Stop the client, the complete listener must be invoked.
-        client.stop();
+            client.stop();
 
-        assertTrue(completeLatch.await(5, TimeUnit.SECONDS));
+            assertTrue(completeLatch.await(5, TimeUnit.SECONDS));
+        }
+        finally
+        {
+            // Release the buffer.
+            Callback callback = callbackRef.get();
+            if (callback != null)
+                callback.succeeded();
+        }
     }
 
     @Test
@@ -757,6 +772,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
             {
                 Content.Sink.write(response, false, UTF_8.encode("A"));
                 Content.Sink.write(response, true, UTF_8.encode("B"));
+                callback.succeeded();
                 return true;
             }
         });

jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java

@@ -13,7 +13,6 @@
 
 package org.eclipse.jetty.test.client.transport;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.lang.management.ManagementFactory;
 import java.net.URI;
@@ -28,7 +27,6 @@ import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
 import javax.management.MBeanServer;
 
-import com.sun.management.HotSpotDiagnosticMXBean;
 import org.awaitility.Awaitility;
 import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
 import org.eclipse.jetty.client.HttpClient;
@@ -158,22 +156,26 @@ public class AbstractTest
         }
     }
 
-    private static void dumpHeap(String testMethodName) throws IOException
+    private static void dumpHeap(String testMethodName) throws Exception
     {
         Path targetDir = Path.of("target/leaks");
-        try (Stream<Path> stream = Files.walk(targetDir))
+        if (Files.exists(targetDir))
         {
-            stream.sorted(Comparator.reverseOrder())
+            try (Stream<Path> stream = Files.walk(targetDir))
-                .map(Path::toFile)
+            {
-                .forEach(java.io.File::delete);
+                stream.sorted(Comparator.reverseOrder())
+                    .map(Path::toFile)
+                    .forEach(java.io.File::delete);
+            }
         }
         Files.createDirectories(targetDir);
         String dumpName = targetDir.resolve(testMethodName + ".hprof").toString();
 
         MBeanServer server = ManagementFactory.getPlatformMBeanServer();
-        HotSpotDiagnosticMXBean mxBean = ManagementFactory.newPlatformMXBeanProxy(
+        Class<?> mxBeanClass = Class.forName("com.sun.management.HotSpotDiagnosticMXBean");
-            server, "com.sun.management:type=HotSpotDiagnostic", HotSpotDiagnosticMXBean.class);
+        Object mxBean = ManagementFactory.newPlatformMXBeanProxy(
-        mxBean.dumpHeap(dumpName, true);
+            server, "com.sun.management:type=HotSpotDiagnostic", mxBeanClass);
+        mxBeanClass.getMethod("dumpHeap", String.class, boolean.class).invoke(mxBean, dumpName, true);
     }
 
     protected void start(Transport transport, Handler handler) throws Exception