An ability to ignore well-known leaks in LeakHunter added

It allows to keep the LeakHunter-based checks enabled while "skipping" some well-known leaks.
JetBrains · Sep 10, 2024 · af97ce1 · af97ce1
1 parent 3a7a28a
commit af97ce1
Show file tree

Hide file tree

Showing 9 changed files with 154 additions and 9 deletions.
diff --git a/platform/core-impl/src/com/intellij/util/ref/DebugReflectionUtil.java b/platform/core-impl/src/com/intellij/util/ref/DebugReflectionUtil.java
@@ -229,7 +229,7 @@ public static class BackLink<V> {
      * when null, it can be computed from field.getName()
      */
     private final String fieldName;
-    private final BackLink<?> backLink;
+    final BackLink<?> backLink;
     private final int depth;
 
     BackLink(@NotNull V value, @Nullable Field field, @Nullable String fieldName, @Nullable BackLink<?> backLink) {
@@ -252,6 +252,10 @@ public String toString() {
       return result.toString();
     }
 
+    String getFieldName() {
+      return this.fieldName != null ? this.fieldName : field.getDeclaringClass().getName() + "." + field.getName();
+    }
+
     void print(@NotNull StringBuilder result) {
       String valueStr;
       Object value = this.value;
@@ -270,9 +274,8 @@ void print(@NotNull StringBuilder result) {
         valueStr = "(" + e.getMessage() + " while computing .toString())";
       }
 
-      Field field = this.field;
-      String fieldName = this.fieldName != null ? this.fieldName : field.getDeclaringClass().getName() + "." + field.getName();
-      result.append("via '").append(fieldName).append("'; Value: '").append(valueStr).append("' of ").append(value.getClass()).append("\n");
+      result.append("via '").append(getFieldName()).append("'; Value: '").append(valueStr).append("' of ").append(value.getClass())
+        .append("\n");
     }
   }
 }
diff --git a/platform/core-impl/src/com/intellij/util/ref/IgnoredTraverseEntry.java b/platform/core-impl/src/com/intellij/util/ref/IgnoredTraverseEntry.java
@@ -0,0 +1,8 @@
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+package com.intellij.util.ref;
+
+import java.util.function.Predicate;
+
+public interface IgnoredTraverseEntry extends
+                                      Predicate<DebugReflectionUtil.BackLink<?>> {
+}
diff --git a/platform/core-impl/src/com/intellij/util/ref/IgnoredTraverseReference.java b/platform/core-impl/src/com/intellij/util/ref/IgnoredTraverseReference.java
@@ -0,0 +1,48 @@
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+package com.intellij.util.ref;
+
+import kotlin.NotImplementedError;
+import org.jetbrains.annotations.NotNull;
+
+public class IgnoredTraverseReference implements IgnoredTraverseEntry {
+
+
+  private final String myField;
+  private final int myIndex;
+
+  /**
+   * Constructs a rule that allows to ignore some known leakage path. It allows to keep the LeakHunter-based checks enabled while "skipping"
+   * some well-known leaks.
+   *
+   * @param field field name in the CLASS_FQN.field_name format
+   * @param index index of the {@param field} frame. Index can be positive and negative. If it's positive it will be enumerated starting
+   *              from the traverse root, from the traverse leaf if it's negative.
+   */
+  public IgnoredTraverseReference(@NotNull final String field, int index) {
+    myField = field;
+    myIndex = index;
+  }
+
+  @Override
+  public boolean test(@NotNull final DebugReflectionUtil.BackLink<?> link) {
+    if (myIndex >= 0) {
+      throw new NotImplementedError("Handling of positive indexes is not implemented yet");
+    }
+    return checkNegativeIndex(link);
+  }
+
+  private boolean checkNegativeIndex(@NotNull final DebugReflectionUtil.BackLink<?> link) {
+    int currentIndex = myIndex;
+    DebugReflectionUtil.BackLink<?> backLink = link;
+
+    while (currentIndex < -1) {
+      backLink = backLink.backLink;
+      currentIndex++;
+      if (backLink == null) {
+        return false;
+      }
+    }
+
+    return myField.equals(backLink.getFieldName());
+  }
+}
diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/util.kt b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/util.kt
@@ -38,7 +38,7 @@ fun assertReferenced(root: Any, referenced: Any) {
   val rootSupplier: Supplier<Map<Any, String>> = Supplier {
     mapOf(root to "root")
   }
-  LeakHunter.processLeaks(rootSupplier, referenced.javaClass, null) { leaked, _ ->
+  LeakHunter.processLeaks(rootSupplier, referenced.javaClass, null, null) { leaked, _ ->
     foundObjects.add(leaked)
     true
   }

diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/util/LeakHunterIgnoreReferencesTest.kt b/platform/platform-tests/testSrc/com/intellij/openapi/util/LeakHunterIgnoreReferencesTest.kt
@@ -0,0 +1,37 @@
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+package com.intellij.openapi.util
+
+import com.intellij.openapi.Disposable
+import com.intellij.testFramework.LeakHunter
+import com.intellij.testFramework.common.timeoutRunBlocking
+import com.intellij.testFramework.junit5.TestApplication
+import com.intellij.util.ref.IgnoredTraverseEntry
+import com.intellij.util.ref.IgnoredTraverseReference
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertDoesNotThrow
+import org.junit.jupiter.api.assertThrows
+import java.util.function.Supplier
+import kotlin.time.Duration.Companion.seconds
+
+@TestApplication
+class LeakHunterIgnoreReferencesTest {
+  @Test
+  fun `leaked project ignored`(): Unit = timeoutRunBlocking(60.seconds) {
+    val ref = ReferenceToDisposable(TestDisposable())
+    assertThrows<AssertionError> { checkReferenced(ref, listOf()) }
+    assertDoesNotThrow { checkReferenced(ref, listOf(IgnoredTraverseReference("com.intellij.openapi.util.ReferenceToDisposable.ref", -1))) }
+  }
+}
+
+fun checkReferenced(root: Any, ignoredTraverseEntries: List<IgnoredTraverseEntry>) {
+  val rootSupplier: Supplier<Map<Any, String>> = Supplier {
+    mapOf(root to "root")
+  }
+  LeakHunter.checkLeak(rootSupplier, TestDisposable::class.java, ignoredTraverseEntries) { true }
+}
+
+class TestDisposable : Disposable {
+  override fun dispose() {}
+}
+
+class ReferenceToDisposable(val ref: Disposable)
diff --git a/platform/testFramework/common/src/LeakHunter.java b/platform/testFramework/common/src/LeakHunter.java
@@ -21,6 +21,7 @@
 import com.intellij.util.io.PersistentEnumeratorCache;
 import com.intellij.util.ref.DebugReflectionUtil;
 import com.intellij.util.ref.GCUtil;
+import com.intellij.util.ref.IgnoredTraverseEntry;
 import com.intellij.util.ui.UIUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -49,6 +50,11 @@ public static void checkNonDefaultProjectLeak() {
     checkLeak(allRoots(), ProjectImpl.class, project -> !project.isDefault());
   }
 
+  @TestOnly
+  public static void checkNonDefaultProjectLeakWithIgnoredEntries(@NotNull List<IgnoredTraverseEntry> ignoredTraverseEntries) {
+    checkLeak(allRoots(), ProjectImpl.class, ignoredTraverseEntries, project -> !project.isDefault());
+  }
+
   @TestOnly
   public static void checkLeak(@NotNull Object root, @NotNull Class<?> suspectClass) throws AssertionError {
     checkLeak(root, suspectClass, null);
@@ -61,7 +67,30 @@ public static void checkLeak(@NotNull Object root, @NotNull Class<?> suspectClas
   public static <T> void checkLeak(@NotNull Supplier<? extends Map<Object, String>> rootsSupplier,
                                    @NotNull Class<T> suspectClass,
                                    @Nullable Predicate<? super T> isReallyLeak) throws AssertionError {
-    processLeaks(rootsSupplier, suspectClass, isReallyLeak, (leaked, backLink)->{
+    processLeaks(rootsSupplier, suspectClass, isReallyLeak, null, (leaked, backLink)->{
+      String message = getLeakedObjectDetails(leaked, backLink, true);
+
+      System.out.println(message);
+      System.out.println(";-----");
+      ThreadUtil.printThreadDump();
+
+      throw new AssertionError(message);
+    });
+  }
+
+  @TestOnly
+  public static <T> void checkLeak(@NotNull Supplier<? extends Map<Object, String>> rootsSupplier,
+                                   @NotNull Class<T> suspectClass,
+                                   @NotNull List<IgnoredTraverseEntry> ignoredTraverseEntries,
+                                   @Nullable Predicate<? super T> isReallyLeak) throws AssertionError {
+    processLeaks(rootsSupplier, suspectClass, isReallyLeak, (backLink) -> {
+      for (IgnoredTraverseEntry entry : ignoredTraverseEntries) {
+        if (entry.test(backLink)) {
+          return true;
+        }
+      }
+      return false;
+    }, (leaked, backLink) -> {
       String message = getLeakedObjectDetails(leaked, backLink, true);
 
       System.out.println(message);
@@ -79,6 +108,7 @@ public static <T> void checkLeak(@NotNull Supplier<? extends Map<Object, String>
   public static <T> void processLeaks(@NotNull Supplier<? extends Map<Object, String>> rootsSupplier,
                                       @NotNull Class<T> suspectClass,
                                       @Nullable Predicate<? super T> isReallyLeak,
+                                      @Nullable Predicate<DebugReflectionUtil.BackLink<?>> leakBackLinkProcessor,
                                       @NotNull PairProcessor<? super T, Object> processor) throws AssertionError {
     if (SwingUtilities.isEventDispatchThread()) {
       UIUtil.dispatchAllInvocationEvents();
@@ -92,6 +122,9 @@ public static <T> void processLeaks(@NotNull Supplier<? extends Map<Object, Stri
     Runnable runnable = () -> {
       try (AccessToken ignored = ProhibitAWTEvents.start("checking for leaks")) {
         DebugReflectionUtil.walkObjects(10000, rootsSupplier.get(), suspectClass, __ -> true, (leaked, backLink) -> {
+          if (leakBackLinkProcessor != null && leakBackLinkProcessor.test(backLink)) {
+            return true;
+          }
           if (isReallyLeak == null || isReallyLeak.test(leaked)) {
             return processor.process(leaked, backLink);
           }

diff --git a/platform/testFramework/common/src/common/testApplication.kt b/platform/testFramework/common/src/common/testApplication.kt
@@ -66,6 +66,7 @@ import com.intellij.util.WalkingState
 import com.intellij.util.concurrency.AppScheduledExecutorService
 import com.intellij.util.indexing.FileBasedIndex
 import com.intellij.util.indexing.FileBasedIndexImpl
+import com.intellij.util.ref.IgnoredTraverseEntry
 import com.intellij.util.ui.EDT
 import com.intellij.util.ui.EdtInvocationManager
 import com.jetbrains.JBR
@@ -326,8 +327,14 @@ fun Application.cleanupApplicationCaches() {
 @TestOnly
 @Internal
 fun assertNonDefaultProjectsAreNotLeaked() {
+  assertNonDefaultProjectsAreNotLeaked(emptyList())
+}
+
+@TestOnly
+@Internal
+fun assertNonDefaultProjectsAreNotLeaked(ignoredTraverseEntries : List<IgnoredTraverseEntry>) {
   try {
-    LeakHunter.checkNonDefaultProjectLeak()
+    LeakHunter.checkNonDefaultProjectLeakWithIgnoredEntries(ignoredTraverseEntries)
   }
   catch (e: AssertionError) {
     publishHeapDump(LEAKED_PROJECTS)

diff --git a/platform/testFramework/src/com/intellij/project/TestProjectManager.kt b/platform/testFramework/src/com/intellij/project/TestProjectManager.kt
@@ -278,6 +278,7 @@ private fun reportLeakedProjects(leakedProjects: Iterable<Project>) {
   val dumpPath = publishHeapDump(LEAKED_PROJECTS)
   LeakHunter.processLeaks(LeakHunter.allRoots(), ProjectImpl::class.java,
                           { hashCodes.contains(System.identityHashCode(it)) },
+                          null,
                           { leaked, backLink ->
                             val hashCode = System.identityHashCode(leaked)
                             message += LeakHunter.getLeakedObjectDetails(leaked, backLink, false)

diff --git a/platform/testFramework/src/com/intellij/testFramework/TestApplicationManager.kt b/platform/testFramework/src/com/intellij/testFramework/TestApplicationManager.kt
@@ -15,7 +15,10 @@ import com.intellij.ide.structureView.StructureViewFactory
 import com.intellij.ide.structureView.impl.StructureViewFactoryImpl
 import com.intellij.openapi.Disposable
 import com.intellij.openapi.actionSystem.DataProvider
-import com.intellij.openapi.application.*
+import com.intellij.openapi.application.Application
+import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.application.WriteAction
+import com.intellij.openapi.application.WriteIntentReadAction
 import com.intellij.openapi.application.impl.ApplicationImpl
 import com.intellij.openapi.command.WriteCommandAction
 import com.intellij.openapi.command.impl.UndoManagerImpl
@@ -39,6 +42,7 @@ import com.intellij.testFramework.common.*
 import com.intellij.util.concurrency.AppExecutorUtil
 import com.intellij.util.concurrency.AppScheduledExecutorService
 import com.intellij.util.ref.GCUtil
+import com.intellij.util.ref.IgnoredTraverseEntry
 import com.intellij.util.ui.EDT
 import com.intellij.util.ui.UIUtil
 import com.intellij.workspaceModel.ide.impl.legacyBridge.module.roots.ModuleRootComponentBridge
@@ -179,6 +183,10 @@ class TestApplicationManager private constructor() {
      */
     @JvmStatic
     fun disposeApplicationAndCheckForLeaks() {
+      disposeApplicationAndCheckForLeaks(emptyList())
+    }
+    @JvmStatic
+    fun disposeApplicationAndCheckForLeaks(ignoredTraverseEntries : List<IgnoredTraverseEntry>) {
       val edtThrowable = runInEdtAndGet {
         runAllCatching(
           { PlatformTestUtil.cleanupAllProjects() },
@@ -194,7 +202,7 @@ class TestApplicationManager private constructor() {
           { UsefulTestCase.waitForAppLeakingThreads(10, TimeUnit.SECONDS) },
           {
             if (ApplicationManager.getApplication() != null) {
-              assertNonDefaultProjectsAreNotLeaked()
+              assertNonDefaultProjectsAreNotLeaked(ignoredTraverseEntries)
             }
           },
           {