Make ResourceLeakDetector.track not final (#15620)

yawkat · normanmaurer · web-flow · commit d28a0fc6598b · 2025-09-04T17:12:35.000+02:00
Motivation:

For fuzzing, paranoid leak detection is necessary, but running GC for
each test is too slow. Instead, I check the `allLeaks` field after each
run for any open trackers (with some pruning to avoid static buffers).
This means that a lot of the tracked data, and in particular the whole
reference queue, is unnecessary.

For better performance, it would be nice to instead only track the
number of open leaks, and compare that number before and after a test
runs. This can be achieved using a custom ResourceLeakTracker
implementation, but for that ResourceLeakDetector.track must become
non-final.

Modification:

Make the method non-final.

Also fix a bunch of warnings.

Result:

ResourceLeakDetector.track can be overridden for more efficient leak
detection.

---------

Co-authored-by: Norman Maurer &lt;norman_maurer@apple.com&gt;
diff --git a/common/src/main/java/io/netty/util/ResourceLeakDetector.java b/common/src/main/java/io/netty/util/ResourceLeakDetector.java
@@ -205,7 +205,8 @@ public ResourceLeakDetector(String resourceType) {
      * @param maxActive This is deprecated and will be ignored.
      */
     @Deprecated
-    public ResourceLeakDetector(Class<?> resourceType, int samplingInterval, long maxActive) {
+    public ResourceLeakDetector(
+            Class<?> resourceType, int samplingInterval, @SuppressWarnings("unused") long maxActive) {
         this(resourceType, samplingInterval);
     }
 
@@ -225,7 +226,8 @@ public ResourceLeakDetector(Class<?> resourceType, int samplingInterval) {
      * @param maxActive This is deprecated and will be ignored.
      */
     @Deprecated
-    public ResourceLeakDetector(String resourceType, int samplingInterval, long maxActive) {
+    public ResourceLeakDetector(
+            String resourceType, int samplingInterval, @SuppressWarnings("unused") long maxActive) {
         this.resourceType = ObjectUtil.checkNotNull(resourceType, "resourceType");
         this.samplingInterval = samplingInterval;
     }
@@ -248,40 +250,37 @@ public final ResourceLeak open(T obj) {
      *
      * @return the {@link ResourceLeakTracker} or {@code null}
      */
-    @SuppressWarnings("unchecked")
-    public final ResourceLeakTracker<T> track(T obj) {
+    public ResourceLeakTracker<T> track(T obj) {
         return track0(obj, false);
     }
 
     /**
      * Creates a new {@link ResourceLeakTracker} which is expected to be closed via
      * {@link ResourceLeakTracker#close(Object)} when the related resource is deallocated.
-     *
+     * <p>
      * Unlike {@link #track(Object)}, this method always returns a tracker, regardless
      * of the detection settings.
      *
      * @return the {@link ResourceLeakTracker}
      */
-    @SuppressWarnings("unchecked")
     public ResourceLeakTracker<T> trackForcibly(T obj) {
         return track0(obj, true);
     }
 
-    @SuppressWarnings("unchecked")
-    private DefaultResourceLeak track0(T obj, boolean force) {
+    private DefaultResourceLeak<T> track0(T obj, boolean force) {
         Level level = ResourceLeakDetector.level;
         if (force ||
                 level == Level.PARANOID ||
                 (level != Level.DISABLED && ThreadLocalRandom.current().nextInt(samplingInterval) == 0)) {
             reportLeak();
-            return new DefaultResourceLeak(obj, refQueue, allLeaks, getInitialHint(resourceType));
+            return new DefaultResourceLeak<>(obj, refQueue, allLeaks, getInitialHint(resourceType));
         }
         return null;
     }
 
     private void clearRefQueue() {
         for (;;) {
-            DefaultResourceLeak ref = (DefaultResourceLeak) refQueue.poll();
+            DefaultResourceLeak<?> ref = (DefaultResourceLeak<?>) refQueue.poll();
             if (ref == null) {
                 break;
             }
@@ -307,7 +306,7 @@ private void reportLeak() {
 
         // Detect and report previous leaks.
         for (;;) {
-            DefaultResourceLeak ref = (DefaultResourceLeak) refQueue.poll();
+            DefaultResourceLeak<?> ref = (DefaultResourceLeak<?>) refQueue.poll();
             if (ref == null) {
                 break;
             }
@@ -391,12 +390,12 @@ public interface LeakListener {
     private static final class DefaultResourceLeak<T>
             extends WeakReference<Object> implements ResourceLeakTracker<T>, ResourceLeak {
 
-        @SuppressWarnings("unchecked") // generics and updaters do not mix.
+        @SuppressWarnings({"unchecked", "rawtypes"}) // generics and updaters do not mix.
         private static final AtomicReferenceFieldUpdater<DefaultResourceLeak<?>, TraceRecord> headUpdater =
                 (AtomicReferenceFieldUpdater)
                         AtomicReferenceFieldUpdater.newUpdater(DefaultResourceLeak.class, TraceRecord.class, "head");
 
-        @SuppressWarnings("unchecked") // generics and updaters do not mix.
+        @SuppressWarnings({"unchecked", "rawtypes"}) // generics and updaters do not mix.
         private static final AtomicIntegerFieldUpdater<DefaultResourceLeak<?>> droppedRecordsUpdater =
                 (AtomicIntegerFieldUpdater)
                         AtomicIntegerFieldUpdater.newUpdater(DefaultResourceLeak.class, "droppedRecords");
@@ -460,7 +459,7 @@ public void record(Object hint) {
          * {@link #TARGET_RECORDS} accesses, backoff occurs. This matches typical access patterns,
          * where there are either a high number of accesses (i.e. a cached buffer), or low (an ephemeral buffer), but
          * not many in between.
-         *
+         * <p>
          * The use of atomics avoids serializing a high number of accesses, when most of the records will be thrown
          * away. High contention only happens when there are very few existing records, which is only likely when the
          * object isn't shared! If this is a problem, the loop can be aborted and the record dropped, because another
@@ -548,6 +547,7 @@ public boolean close(T trackedObject) {
          * @param ref the reference. If {@code null}, this method has no effect.
          * @see java.lang.ref.Reference#reachabilityFence
          */
+        @SuppressWarnings({"SynchronizationOnLocalVariableOrMethodParameter", "EmptySynchronizedStatement"})
         private static void reachabilityFence0(Object ref) {
             if (ref != null) {
                 synchronized (ref) {
@@ -591,7 +591,7 @@ private String generateReport(TraceRecord oldHead) {
             buf.append("Recent access records: ").append(NEWLINE);
 
             int i = 1;
-            Set<String> seen = new HashSet<String>(present);
+            Set<String> seen = new HashSet<>(present);
             for (; oldHead != TraceRecord.BOTTOM; oldHead = oldHead.next) {
                 String s = oldHead.toString();
                 if (seen.add(s)) {
@@ -629,10 +629,10 @@ private String generateReport(TraceRecord oldHead) {
     }
 
     private static final AtomicReference<String[]> excludedMethods =
-            new AtomicReference<String[]>(EmptyArrays.EMPTY_STRINGS);
+            new AtomicReference<>(EmptyArrays.EMPTY_STRINGS);
 
-    public static void addExclusions(Class clz, String ... methodNames) {
-        Set<String> nameSet = new HashSet<String>(Arrays.asList(methodNames));
+    public static void addExclusions(@SuppressWarnings("rawtypes") Class clz, String ... methodNames) {
+        Set<String> nameSet = new HashSet<>(Arrays.asList(methodNames));
         // Use loop rather than lookup. This avoids knowing the parameters, and doesn't have to handle
         // NoSuchMethodException.
         for (Method method : clz.getDeclaredMethods()) {
