#10226 handle review comments

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

jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java

@@ -85,9 +85,9 @@ public void dispose() throws Exception
     {
         LifeCycle.stop(client);
         LifeCycle.stop(server);
-        Set<ArrayByteBufferPool.Tracking.TrackingBuffer> serverLeaks = serverBufferPool.getLeaks();
+        Set<ArrayByteBufferPool.Tracking.Buffer> serverLeaks = serverBufferPool.getLeaks();
         assertEquals(0, serverLeaks.size(), serverBufferPool.dumpLeaks());
-        Set<ArrayByteBufferPool.Tracking.TrackingBuffer> clientLeaks = clientBufferPool.getLeaks();
+        Set<ArrayByteBufferPool.Tracking.Buffer> clientLeaks = clientBufferPool.getLeaks();
         assertEquals(0, clientLeaks.size(), clientBufferPool.dumpLeaks());
     }
 

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

@@ -22,8 +22,6 @@
           <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>

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

@@ -75,32 +75,26 @@ public void dispose()
         try
         {
             if (serverBufferPool != null)
-            {
-                try
-                {
-                    Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> serverBufferPool.getLeaks().size(), Matchers.is(0));
-                }
-                catch (Exception e)
-                {
-                    fail(e.getMessage() + "\n---\nServer Leaks: " + serverBufferPool.dumpLeaks() + "---\n");
-                }
-            }
+                assertNoLeaks(serverBufferPool, "\n---\nServer Leaks: " + serverBufferPool.dumpLeaks() + "---\n");
             if (clientBufferPool != null)
-            {
-                try
-                {
-                    Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> clientBufferPool.getLeaks().size(), Matchers.is(0));
-                }
-                catch (Exception e)
-                {
-                    fail(e.getMessage() + "\n---\nClient Leaks: " + clientBufferPool.dumpLeaks() + "---\n");
-                }
-            }
+                assertNoLeaks(clientBufferPool, "\n---\nClient Leaks: " + clientBufferPool.dumpLeaks() + "---\n");
         }
         finally
         {
             LifeCycle.stop(client);
             LifeCycle.stop(server);
         }
     }
+
+    private void assertNoLeaks(ArrayByteBufferPool.Tracking bufferPool, String msg)
+    {
+        try
+        {
+            Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> bufferPool.getLeaks().size(), Matchers.is(0));
+        }
+        catch (Exception e)
+        {
+            fail(e.getMessage() + msg);
+        }
+    }
 }

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

@@ -239,8 +239,7 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett
                 Content.Sink.write(response, false, UTF_8.encode(paramValue1));
                 String paramValue2 = fields.getValue(paramName2);
                 assertEquals("", paramValue2);
-                Content.Sink.write(response, true, UTF_8.encode("empty"));
-                callback.succeeded();
+                Content.Sink.write(response, true, "empty", callback);
                 return true;
             }
         });
@@ -274,8 +273,7 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett
                     Content.Sink.write(response, false, UTF_8.encode(paramValue));
                 }
                 String paramValue2 = fields.getValue(paramName2);
-                Content.Sink.write(response, true, UTF_8.encode(paramValue2));
-                callback.succeeded();
+                Content.Sink.write(response, true, paramValue2, callback);
                 return true;
             }
         });
@@ -771,8 +769,7 @@ public void testSmallAsyncContent() throws Exception
             public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception
             {
                 Content.Sink.write(response, false, UTF_8.encode("A"));
-                Content.Sink.write(response, true, UTF_8.encode("B"));
-                callback.succeeded();
+                Content.Sink.write(response, true, "B", callback);
                 return true;
             }
         });

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java

@@ -577,14 +577,14 @@ public Quadratic(int minCapacity, int maxCapacity, int maxBucketSize, long maxHe
      * <p>A variant of {@link ArrayByteBufferPool} that tracks buffer
      * acquires/releases, useful to identify buffer leaks.</p>
      * <p>Use {@link #getLeaks()} when the system is idle to get
-     * the {@link TrackingBuffer}s that have been leaked, which contain
+     * the {@link Buffer}s that have been leaked, which contain
      * the stack trace information of where the buffer was acquired.</p>
      */
     public static class Tracking extends ArrayByteBufferPool
     {
         private static final Logger LOG = LoggerFactory.getLogger(Tracking.class);
 
-        private final Set<TrackingBuffer> buffers = ConcurrentHashMap.newKeySet();
+        private final Set<Buffer> buffers = ConcurrentHashMap.newKeySet();
 
         public Tracking()
         {
@@ -605,34 +605,34 @@ public Tracking(int minCapacity, int maxCapacity, int maxBucketSize, long maxHea
         public RetainableByteBuffer acquire(int size, boolean direct)
         {
             RetainableByteBuffer buffer = super.acquire(size, direct);
-            TrackingBuffer wrapper = new TrackingBuffer(buffer, size);
+            Buffer wrapper = new Buffer(buffer, size);
             if (LOG.isDebugEnabled())
                 LOG.debug("acquired {}", wrapper);
             buffers.add(wrapper);
             return wrapper;
         }
 
-        public Set<TrackingBuffer> getLeaks()
+        public Set<Buffer> getLeaks()
         {
             return buffers;
         }
 
         public String dumpLeaks()
         {
             return getLeaks().stream()
-                .map(TrackingBuffer::dump)
+                .map(Buffer::dump)
                 .collect(Collectors.joining(System.lineSeparator()));
         }
 
-        public class TrackingBuffer extends RetainableByteBuffer.Wrapper
+        public class Buffer extends RetainableByteBuffer.Wrapper
         {
             private final int size;
             private final Instant acquireInstant;
             private final Throwable acquireStack;
             private final List<Throwable> retainStacks = new CopyOnWriteArrayList<>();
             private final List<Throwable> releaseStacks = new CopyOnWriteArrayList<>();
 
-            private TrackingBuffer(RetainableByteBuffer wrapped, int size)
+            private Buffer(RetainableByteBuffer wrapped, int size)
             {
                 super(wrapped);
                 this.size = size;

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java

@@ -14,12 +14,12 @@
 package org.eclipse.jetty.server.handler;
 
 import java.io.ByteArrayOutputStream;
-import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.HttpURLConnection;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import java.security.KeyStore;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.HttpsURLConnection;
@@ -52,6 +52,8 @@ public class DebugHandlerTest
 
     private SSLContext sslContext;
     private Server server;
+    private ArrayByteBufferPool.Tracking httpTrackingBufferPool;
+    private ArrayByteBufferPool.Tracking sslTrackingBufferPool;
     private URI serverURI;
     private URI secureServerURI;
 
@@ -63,15 +65,17 @@ public void startServer() throws Exception
     {
         server = new Server();
 
-        ServerConnector httpConnector = new ServerConnector(server, null, null, new ArrayByteBufferPool.Tracking(), 1, 1, new HttpConnectionFactory());
+        httpTrackingBufferPool = new ArrayByteBufferPool.Tracking();
+        ServerConnector httpConnector = new ServerConnector(server, null, null, httpTrackingBufferPool, 1, 1, new HttpConnectionFactory());
         httpConnector.setPort(0);
         server.addConnector(httpConnector);
 
-        File keystorePath = MavenTestingUtils.getTestResourceFile("keystore.p12");
+        Path keystorePath = MavenTestingUtils.getTestResourcePath("keystore.p12");
         SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
-        sslContextFactory.setKeyStorePath(keystorePath.getAbsolutePath());
+        sslContextFactory.setKeyStorePath(keystorePath.toAbsolutePath().toString());
         sslContextFactory.setKeyStorePassword("storepwd");
-        ServerConnector sslConnector = new ServerConnector(server, null, null, new ArrayByteBufferPool.Tracking(), 1, 1,
+        sslTrackingBufferPool = new ArrayByteBufferPool.Tracking();
+        ServerConnector sslConnector = new ServerConnector(server, null, null, sslTrackingBufferPool, 1, 1,
             AbstractConnectionFactory.getFactories(sslContextFactory, new HttpConnectionFactory()));
 
         server.addConnector(sslConnector);
@@ -128,10 +132,10 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
     @AfterEach
     public void stopServer() throws Exception
     {
-        ArrayByteBufferPool.Tracking byteBufferPool = (ArrayByteBufferPool.Tracking)server.getConnectors()[0].getByteBufferPool();
         try
         {
-            assertThat("Server Leaks: " + byteBufferPool.dumpLeaks(), byteBufferPool.getLeaks().size(), Matchers.is(0));
+            assertThat("Server HTTP Leaks: " + httpTrackingBufferPool.dumpLeaks(), httpTrackingBufferPool.getLeaks().size(), Matchers.is(0));
+            assertThat("Server SSL Leaks: " + sslTrackingBufferPool.dumpLeaks(), sslTrackingBufferPool.getLeaks().size(), Matchers.is(0));
         }
         finally
         {

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/ServerConnectorSslServerTest.java

@@ -60,6 +60,7 @@
 public class ServerConnectorSslServerTest extends HttpServerTestBase
 {
     private SSLContext _sslContext;
+    private ArrayByteBufferPool.Tracking trackingBufferPool;
 
     public ServerConnectorSslServerTest()
     {
@@ -73,10 +74,10 @@ public void init() throws Exception
         SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
         sslContextFactory.setKeyStorePath(keystorePath);
         sslContextFactory.setKeyStorePassword("storepwd");
-        ArrayByteBufferPool.Tracking pool = new ArrayByteBufferPool.Tracking();
+        trackingBufferPool = new ArrayByteBufferPool.Tracking();
 
         HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory();
-        ServerConnector connector = new ServerConnector(_server, null, null, pool, 1, 1, AbstractConnectionFactory.getFactories(sslContextFactory, httpConnectionFactory));
+        ServerConnector connector = new ServerConnector(_server, null, null, trackingBufferPool, 1, 1, AbstractConnectionFactory.getFactories(sslContextFactory, httpConnectionFactory));
         SecureRequestCustomizer secureRequestCustomer = new SecureRequestCustomizer();
         secureRequestCustomer.setSslSessionAttribute("SSL_SESSION");
         httpConnectionFactory.getHttpConfiguration().addCustomizer(secureRequestCustomer);
@@ -110,8 +111,7 @@ public void init() throws Exception
     @AfterEach
     public void dispose() throws Exception
     {
-        ArrayByteBufferPool.Tracking byteBufferPool = (ArrayByteBufferPool.Tracking)_server.getConnectors()[0].getByteBufferPool();
-        assertThat("Server Leaks: " + byteBufferPool.dumpLeaks(), byteBufferPool.getLeaks().size(), Matchers.is(0));
+        assertThat("Server Leaks: " + trackingBufferPool.dumpLeaks(), trackingBufferPool.getLeaks().size(), Matchers.is(0));
     }
 
     @Override

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

@@ -126,31 +126,9 @@ public void dispose(TestInfo testInfo) throws Exception
         try
         {
             if (serverBufferPool != null && !isLeakTrackingDisabled(testInfo, "server"))
-            {
-                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");
-                }
-            }
+                assertNoLeaks(serverBufferPool, testInfo, "server-", "\n---\nServer Leaks: " + serverBufferPool.dumpLeaks() + "---\n");
             if (clientBufferPool != null && !isLeakTrackingDisabled(testInfo, "client"))
-            {
-                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");
-                }
-            }
+                assertNoLeaks(clientBufferPool, testInfo, "client-", "\n---\nClient Leaks: " + clientBufferPool.dumpLeaks() + "---\n");
         }
         finally
         {
@@ -159,6 +137,20 @@ public void dispose(TestInfo testInfo) throws Exception
         }
     }
 
+    private void assertNoLeaks(ArrayByteBufferPool.Tracking bufferPool, TestInfo testInfo, String prefix, String msg) throws Exception
+    {
+        try
+        {
+            Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> bufferPool.getLeaks().size(), Matchers.is(0));
+        }
+        catch (Exception e)
+        {
+            String className = testInfo.getTestClass().orElseThrow().getName();
+            dumpHeap(prefix + className + msg);
+            fail(e.getMessage());
+        }
+    }
+
     private static boolean isLeakTrackingDisabled(TestInfo testInfo, String tagSubValue)
     {
         String disableLeakTrackingTagValue = "DisableLeakTracking";