#10226 - improve ArrayByteBufferPool.Tracking

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
jetty · Aug 23, 2023 · 1429cb2 · 1429cb2
1 parent 7340b0a
commit 1429cb2
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 17 deletions.
diff --git a/...core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java b/...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.Buffer> serverLeaks = serverBufferPool.getLeaks();
+        Set<ArrayByteBufferPool.Tracking.TrackingBuffer> serverLeaks = serverBufferPool.getLeaks();
         assertEquals(0, serverLeaks.size(), serverBufferPool.dumpLeaks());
-        Set<ArrayByteBufferPool.Tracking.Buffer> clientLeaks = clientBufferPool.getLeaks();
+        Set<ArrayByteBufferPool.Tracking.TrackingBuffer> clientLeaks = clientBufferPool.getLeaks();
         assertEquals(0, clientLeaks.size(), clientBufferPool.dumpLeaks());
     }
 

diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java
@@ -19,8 +19,10 @@
 import java.nio.ByteBuffer;
 import java.time.Instant;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Consumer;
 import java.util.function.IntUnaryOperator;
@@ -575,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 Buffer}s that have been leaked, which contain
+     * the {@link TrackingBuffer}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<Buffer> buffers = ConcurrentHashMap.newKeySet();
+        private final Set<TrackingBuffer> buffers = ConcurrentHashMap.newKeySet();
 
         public Tracking()
         {
@@ -603,32 +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);
-            Buffer wrapper = new Buffer(buffer, size);
+            TrackingBuffer wrapper = new TrackingBuffer(buffer, size);
             if (LOG.isDebugEnabled())
                 LOG.debug("acquired {}", wrapper);
             buffers.add(wrapper);
             return wrapper;
         }
 
-        public Set<Buffer> getLeaks()
+        public Set<TrackingBuffer> getLeaks()
         {
             return buffers;
         }
 
         public String dumpLeaks()
         {
             return getLeaks().stream()
-                .map(Buffer::dump)
+                .map(TrackingBuffer::dump)
                 .collect(Collectors.joining(System.lineSeparator()));
         }
 
-        public class Buffer extends RetainableByteBuffer.Wrapper
+        public class TrackingBuffer 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 Buffer(RetainableByteBuffer wrapped, int size)
+            private TrackingBuffer(RetainableByteBuffer wrapped, int size)
             {
                 super(wrapped);
                 this.size = size;
@@ -651,6 +655,13 @@ public Throwable getAcquireStack()
                 return acquireStack;
             }
 
+            @Override
+            public void retain()
+            {
+                super.retain();
+                retainStacks.add(new Throwable());
+            }
+
             @Override
             public boolean release()
             {
@@ -661,20 +672,26 @@ public boolean release()
                     if (LOG.isDebugEnabled())
                         LOG.debug("released {}", this);
                 }
+                releaseStacks.add(new Throwable());
                 return released;
             }
 
             public String dump()
             {
                 StringWriter w = new StringWriter();
-                getAcquireStack().printStackTrace(new PrintWriter(w));
-                return "%s of %d bytes on %s at %s".formatted(getClass().getSimpleName(), getSize(), getAcquireInstant(), w);
-            }
-
-            @Override
-            public String toString()
-            {
-                return "%s@%x[%s]".formatted(getClass().getSimpleName(), hashCode(), super.toString());
+                PrintWriter pw = new PrintWriter(w);
+                getAcquireStack().printStackTrace(pw);
+                pw.println("\n" + retainStacks.size() + " retain(s)");
+                for (Throwable retainStack : retainStacks)
+                {
+                    retainStack.printStackTrace(pw);
+                }
+                pw.println("\n" + releaseStacks.size() + " release(s)");
+                for (Throwable releaseStack : releaseStacks)
+                {
+                    releaseStack.printStackTrace(pw);
+                }
+                return "%s@%x of %d bytes on %s acquired at %s".formatted(getClass().getSimpleName(), hashCode(), getSize(), getAcquireInstant(), w);
             }
         }
     }