From 10bdb6f46b64b19cd182aadda3980a99e841c74c Mon Sep 17 00:00:00 2001 From: Sergei Tachenov Date: Wed, 18 Sep 2024 11:02:32 +0300 Subject: [PATCH] IJPL-162465 Do not dispose selected usages in Show Usages When the Show Usages popup is closed, the smart pointers contained in usage infos are disposed. Then, AFTER the popup is disposed, we navigate to the selected usage(s). But at that time it's already disposed, so some functionality may not work, e.g. WI-79031. Fix by maintaining a set of usage infos that must not be disposed in UsageViewImpl. We update this list along with the list of selected usages. Of course, it's not guaranteed that the selected usages will be used at all, and therefore they may never be disposed. That's OK, as according to SmartPointerManager.removePointer, disposing smart pointers isn't mandatory. And we're talking about a single pointer in the vast majority of cases here. And anyway, in the pre-regression implementations, ALL the selected usages would be copied every time selection changes, and were never disposed after that. So this fix is still better than that, and it doesn't rely on slow ops or the broken copy() implementation. GitOrigin-RevId: 751c9dc8b99d48874680cddef8b1b62bbc525975 --- .../find/actions/ShowUsagesTable.java | 22 ++++++++++++--- .../intellij/usages/impl/UsageViewImpl.java | 27 +++++++++++++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/find/actions/ShowUsagesTable.java b/platform/lang-impl/src/com/intellij/find/actions/ShowUsagesTable.java index 89fbcb7ed9be..7718cebfd390 100644 --- a/platform/lang-impl/src/com/intellij/find/actions/ShowUsagesTable.java +++ b/platform/lang-impl/src/com/intellij/find/actions/ShowUsagesTable.java @@ -24,6 +24,7 @@ import com.intellij.usages.impl.*; import com.intellij.util.concurrency.AppExecutorUtil; import com.intellij.util.concurrency.ThreadingAssertions; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.SmartHashSet; import com.intellij.util.ui.ColumnInfo; import com.intellij.util.ui.JBUI; import com.intellij.util.ui.ListTableModel; @@ -51,9 +52,9 @@ public final class ShowUsagesTable extends JBTable implements UiDataProvider { static final int MIN_COLUMN_WIDTH = 200; private final ShowUsagesTableCellRenderer myRenderer; - private final UsageView myUsageView; + private final UsageViewImpl myUsageView; - ShowUsagesTable(@NotNull ShowUsagesTableCellRenderer renderer, UsageView usageView) { + ShowUsagesTable(@NotNull ShowUsagesTableCellRenderer renderer, UsageViewImpl usageView) { myRenderer = renderer; myUsageView = usageView; ScrollingUtil.installActions(this); @@ -112,6 +113,7 @@ public final class ShowUsagesTable extends JBTable implements UiDataProvider { moreUsagesSelected.set(false); filteredOutUsagesSelected.set(null); java.util.List usages = null; + var nonDisposableUsageInfos = new SmartHashSet(); //todo List for (int i : getSelectedRows()) { Object value = getValueAt(i, 0); @@ -133,11 +135,25 @@ public final class ShowUsagesTable extends JBTable implements UiDataProvider { break; } if (usages == null) usages = new ArrayList<>(); - usages.add(usage instanceof UsageInfo2UsageAdapter ? ((UsageInfo2UsageAdapter)usage).getUsageInfo() : usage); + if (usage instanceof UsageInfo2UsageAdapter adapter) { + var usageInfo = adapter.getUsageInfo(); + usages.add(usageInfo); + nonDisposableUsageInfos.add(usageInfo); + } + else { + usages.add(usage); + } } } selectedUsages.set(usages); + // The callback below is called after the popup is disposed, + // and when it happens, it disposes all smart pointers. + // This prevents some functionality in navigateTo. + // Therefore, we need to preserve the selected usages smart pointers. + // According to com.intellij.psi.SmartPointerManager.removePointer, + // disposing isn't mandatory so there should be no leaks here. + myUsageView.setNonDisposableUsageInfos(nonDisposableUsageInfos); }); return () -> { diff --git a/platform/usageView-impl/src/com/intellij/usages/impl/UsageViewImpl.java b/platform/usageView-impl/src/com/intellij/usages/impl/UsageViewImpl.java index 41328d8794eb..07f5c4b49565 100644 --- a/platform/usageView-impl/src/com/intellij/usages/impl/UsageViewImpl.java +++ b/platform/usageView-impl/src/com/intellij/usages/impl/UsageViewImpl.java @@ -76,6 +76,7 @@ import java.awt.event.MouseEvent; import java.util.List; import java.util.*; import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -116,6 +117,7 @@ public class UsageViewImpl implements UsageViewEx { private volatile boolean isDisposed; private volatile boolean myChangesDetected; private @Nullable GroupNode myAutoSelectedGroupNode; + private final AtomicReference<@NotNull Set> myNonDisposableUsageInfos = new AtomicReference<>(Collections.emptySet()); public static final Comparator USAGE_COMPARATOR_BY_FILE_AND_OFFSET = (o1, o2) -> { if (o1 == o2) return 0; @@ -1619,12 +1621,33 @@ public class UsageViewImpl implements UsageViewEx { } } + /** + * Prevents the specified usage infos from being disposed on view dispose + *

+ * Used by Show Usages, as navigation to the selected usage happens after the popup is disposed. + * Not disposing usage infos is leak-safe, as disposing only involves getting rid of smart pointers, + * which is not mandatory. + * Still, when we have hundreds or thousands of usages, it's beneficial to dispose not needed ones, + * which means not selected ones, which in most cases is all of them but one. + * Therefore, this method is called to specify the selected usages (usually one), which then may + * (but not have to) be used to navigate to the usage(s). + *

+ * @param usageInfos the set of usage infos which won't be disposed with the vew + */ + @ApiStatus.Internal + public void setNonDisposableUsageInfos(@NotNull Set usageInfos) { + myNonDisposableUsageInfos.set(usageInfos); + } + private void disposeSmartPointers() { + var nonDisposableUsageInfos = myNonDisposableUsageInfos.get(); List> smartPointers = new ArrayList<>(); for (Usage usage : myUsageNodes.keySet()) { if (usage instanceof UsageInfo2UsageAdapter) { - SmartPsiElementPointer pointer = ((UsageInfo2UsageAdapter)usage).getUsageInfo().getSmartPointer(); - smartPointers.add(pointer); + var usageInfo = ((UsageInfo2UsageAdapter)usage).getUsageInfo(); + if (!nonDisposableUsageInfos.contains(usageInfo)) { + smartPointers.add(usageInfo.getSmartPointer()); + } } }