From 794799dce90ecc5a0207285aa5c39449eb92b18d Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Thu, 21 Mar 2024 17:55:49 +0100 Subject: [PATCH] make NamedObjectProviderBinding thread-safe to fix EA-1138581 (plugin) - NSEE: SmartList.getFromArray GitOrigin-RevId: 482107e14834d2df8e50ec7f893cffd6c964a996 --- .../reference/NamedObjectProviderBinding.java | 48 ++++++++++++------- .../resolve/reference/ProviderBinding.java | 13 +++-- .../reference/PsiReferenceRegistrarImpl.java | 13 ++++- .../ReferenceProvidersRegistryImpl.java | 10 ++-- .../reference/SimpleProviderBinding.java | 17 ++++--- .../util/containers/ContainerUtil.java | 19 ++++++++ .../psi/UastReferenceByUsageAdapter.kt | 9 ++-- 7 files changed, 88 insertions(+), 41 deletions(-) diff --git a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/NamedObjectProviderBinding.java b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/NamedObjectProviderBinding.java index 87a8afb24381..c8d04cc6c96c 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/NamedObjectProviderBinding.java +++ b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/NamedObjectProviderBinding.java @@ -7,42 +7,46 @@ import com.intellij.patterns.ElementPattern; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiReferenceProvider; import com.intellij.psi.PsiReferenceService; +import com.intellij.util.ArrayUtil; import com.intellij.util.ProcessingContext; import com.intellij.util.SharedProcessingContext; -import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Collection; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * @author maxim */ public abstract class NamedObjectProviderBinding implements ProviderBinding { - private final Map>>> myNamesToProvidersMap = new HashMap<>(5); - private final Map>>> myNamesToProvidersMapInsensitive = new HashMap<>(5); + /** + * arrays inside these maps must be copy-on-write to avoid data races, since they can be read concurrently, + * via {@link #addAcceptableReferenceProviders} + */ + private final Map>[]> myNamesToProvidersMap = new ConcurrentHashMap<>(5); + private final Map>[]> myNamesToProvidersMapInsensitive = new ConcurrentHashMap<>(5); + synchronized public void registerProvider(@NonNls String @NotNull [] names, @NotNull ElementPattern filter, boolean caseSensitive, @NotNull PsiReferenceProvider provider, double priority) { - Map>>> map = caseSensitive ? myNamesToProvidersMap : myNamesToProvidersMapInsensitive; + Map>[]> map = caseSensitive ? myNamesToProvidersMap : myNamesToProvidersMapInsensitive; for (String attributeName : names) { String key = caseSensitive ? attributeName : StringUtil.toLowerCase(attributeName); - List>> psiReferenceProviders = map.get(key); + ProviderInfo>[] psiReferenceProviders = map.get(key); - if (psiReferenceProviders == null) { - map.put(key, psiReferenceProviders = new SmartList<>()); - } + ProviderInfo> newInfo = new ProviderInfo<>(provider, filter, priority); + ProviderInfo>[] newProviders = psiReferenceProviders == null ? new ProviderInfo[]{newInfo} : ArrayUtil.append(psiReferenceProviders, newInfo); - psiReferenceProviders.add(new ProviderInfo<>(provider, filter, priority)); + map.put(key, newProviders); } } @@ -52,21 +56,30 @@ public abstract class NamedObjectProviderBinding implements ProviderBinding { @NotNull PsiReferenceService.Hints hints) { String name = getName(position); if (name != null) { - addMatchingProviders(position, ContainerUtil.notNullize(myNamesToProvidersMap.get(name)), list, hints); - addMatchingProviders(position, ContainerUtil.notNullize(myNamesToProvidersMapInsensitive.get(StringUtil.toLowerCase(name))), list, hints); + addMatchingProviders(position, myNamesToProvidersMap.get(name), list, hints); + addMatchingProviders(position, myNamesToProvidersMapInsensitive.get(StringUtil.toLowerCase(name)), list, hints); } } + synchronized @Override public void unregisterProvider(@NotNull PsiReferenceProvider provider) { - for (List>> list : myNamesToProvidersMap.values()) { - list.removeIf(trinity -> trinity.provider.equals(provider)); + for (Map.Entry>[]> entry : myNamesToProvidersMap.entrySet()) { + entry.setValue((ProviderInfo>[])removeFromArray(provider, entry.getValue())); } - for (List>> list : myNamesToProvidersMapInsensitive.values()) { - list.removeIf(trinity -> trinity.provider.equals(provider)); + for (Map.Entry>[]> entry : myNamesToProvidersMapInsensitive.entrySet()) { + entry.setValue((ProviderInfo>[])removeFromArray(provider, entry.getValue())); } } + static @NotNull ProviderInfo @NotNull [] removeFromArray(@NotNull PsiReferenceProvider provider, @NotNull ProviderInfo @NotNull [] array) { + int i = ContainerUtil.indexOf(array, trinity -> trinity.provider.equals(provider)); + if (i != -1) { + return ArrayUtil.remove(array, i, ProviderInfo.ARRAY_FACTORY); + } + return array; + } + boolean isEmpty() { return myNamesToProvidersMap.isEmpty() && myNamesToProvidersMapInsensitive.isEmpty(); } @@ -74,9 +87,10 @@ public abstract class NamedObjectProviderBinding implements ProviderBinding { protected abstract @Nullable String getName(@NotNull PsiElement position); static void addMatchingProviders(@NotNull PsiElement position, - @NotNull List>> providerList, + @NotNull ProviderInfo> @Nullable [] providerList, @NotNull Collection> output, @NotNull PsiReferenceService.Hints hints) { + if (providerList == null) return; SharedProcessingContext sharedProcessingContext = new SharedProcessingContext(); for (ProviderInfo> info : providerList) { diff --git a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ProviderBinding.java b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ProviderBinding.java index 61c48f87636d..3d07336fd194 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ProviderBinding.java +++ b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ProviderBinding.java @@ -5,18 +5,21 @@ package com.intellij.psi.impl.source.resolve.reference; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiReferenceProvider; import com.intellij.psi.PsiReferenceService; +import com.intellij.util.ArrayFactory; import com.intellij.util.ProcessingContext; import org.jetbrains.annotations.NotNull; import java.util.List; public interface ProviderBinding { - class ProviderInfo { - public final @NotNull PsiReferenceProvider provider; - public final @NotNull Context processingContext; - public final double priority; + class ProviderInfo { + static final ProviderInfo[] EMPTY_ARRAY = new ProviderInfo[0]; + static final ArrayFactory> ARRAY_FACTORY = n -> n == 0 ? ProviderInfo.EMPTY_ARRAY : new ProviderInfo[n]; + final @NotNull PsiReferenceProvider provider; + final @NotNull T processingContext; + final double priority; - public ProviderInfo(@NotNull PsiReferenceProvider provider, @NotNull Context processingContext, double priority) { + ProviderInfo(@NotNull PsiReferenceProvider provider, @NotNull T processingContext, double priority) { this.provider = provider; this.processingContext = processingContext; this.priority = priority; diff --git a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/PsiReferenceRegistrarImpl.java b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/PsiReferenceRegistrarImpl.java index 757504a7a1ac..efdb453ff2c9 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/PsiReferenceRegistrarImpl.java +++ b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/PsiReferenceRegistrarImpl.java @@ -14,9 +14,11 @@ import com.intellij.util.ArrayUtilRt; import com.intellij.util.ProcessingContext; import com.intellij.util.SmartList; import com.intellij.util.containers.ConcurrentFactoryMap; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.Unmodifiable; import java.util.*; import java.util.concurrent.ConcurrentMap; @@ -177,8 +179,9 @@ public class PsiReferenceRegistrarImpl extends PsiReferenceRegistrar { } @ApiStatus.Internal - public @NotNull List> getPairsByElement(@NotNull PsiElement element, - @NotNull PsiReferenceService.Hints hints) { + @Unmodifiable + @NotNull List> getPairsByElement(@NotNull PsiElement element, + @NotNull PsiReferenceService.Hints hints) { ProviderBinding[] bindings = myBindingCache.get(element.getClass()); if (bindings.length == 0) return Collections.emptyList(); @@ -188,4 +191,10 @@ public class PsiReferenceRegistrarImpl extends PsiReferenceRegistrar { } return ret; } + @ApiStatus.Internal + @Unmodifiable + public @NotNull List getPsiReferenceProvidersByElement(@NotNull PsiElement element, + @NotNull PsiReferenceService.Hints hints) { + return ContainerUtil.map(getPairsByElement(element, hints), info -> info.provider); + } } diff --git a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ReferenceProvidersRegistryImpl.java b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ReferenceProvidersRegistryImpl.java index 6b8326f62bb0..2bccadb5e3ef 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ReferenceProvidersRegistryImpl.java +++ b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/ReferenceProvidersRegistryImpl.java @@ -161,17 +161,17 @@ public final class ReferenceProvidersRegistryImpl extends ReferenceProvidersRegi private static @NotNull Double2ObjectMap> mapNotEmptyReferencesFromProviders(@NotNull PsiElement context, @NotNull List> providers) { Double2ObjectOpenHashMap> map = new Double2ObjectOpenHashMap<>(); - for (ProviderBinding.ProviderInfo trinity : providers) { - PsiReference[] refs = getReferences(context, trinity); + for (ProviderBinding.ProviderInfo info : providers) { + PsiReference[] refs = getReferences(context, info); if (refs.length > 0) { - List list = map.get(trinity.priority); + List list = map.get(info.priority); if (list == null) { list = new SmartList<>(); - map.put(trinity.priority, list); + map.put(info.priority, list); } list.add(refs); if (IdempotenceChecker.isLoggingEnabled()) { - IdempotenceChecker.logTrace(trinity.provider + " returned " + Arrays.toString(refs)); + IdempotenceChecker.logTrace(info.provider + " returned " + Arrays.toString(refs)); } } } diff --git a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/SimpleProviderBinding.java b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/SimpleProviderBinding.java index 66a35d8794cd..85b77c646cc6 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/SimpleProviderBinding.java +++ b/platform/core-impl/src/com/intellij/psi/impl/source/resolve/reference/SimpleProviderBinding.java @@ -6,32 +6,37 @@ import com.intellij.patterns.ElementPattern; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiReferenceProvider; import com.intellij.psi.PsiReferenceService; +import com.intellij.util.ArrayUtil; import com.intellij.util.ProcessingContext; -import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import java.util.List; class SimpleProviderBinding implements ProviderBinding { - private final List>> myProviderPairs = ContainerUtil.createLockFreeCopyOnWriteList(); + /** + * the array must be copy-on-write to avoid data races, since it can be read concurrently, via {@link #addAcceptableReferenceProviders} + */ + volatile private @NotNull ProviderInfo @NotNull [] myProviderInfos = ProviderInfo.EMPTY_ARRAY; + synchronized void registerProvider(@NotNull PsiReferenceProvider provider, @NotNull ElementPattern pattern, double priority) { - myProviderPairs.add(new ProviderInfo<>(provider, pattern, priority)); + myProviderInfos = ArrayUtil.append(myProviderInfos, new ProviderInfo<>(provider, pattern, priority), ProviderInfo.ARRAY_FACTORY); } @Override public void addAcceptableReferenceProviders(@NotNull PsiElement position, @NotNull List> list, @NotNull PsiReferenceService.Hints hints) { - NamedObjectProviderBinding.addMatchingProviders(position, myProviderPairs, list, hints); + NamedObjectProviderBinding.addMatchingProviders(position, (ProviderInfo>[])myProviderInfos, list, hints); } @Override + synchronized public void unregisterProvider(@NotNull PsiReferenceProvider provider) { - myProviderPairs.removeIf(trinity -> trinity.provider.equals(provider)); + myProviderInfos = NamedObjectProviderBinding.removeFromArray(provider, myProviderInfos); } boolean isEmpty() { - return myProviderPairs.isEmpty(); + return myProviderInfos.length == 0; } } diff --git a/platform/util/src/com/intellij/util/containers/ContainerUtil.java b/platform/util/src/com/intellij/util/containers/ContainerUtil.java index 16e1a3d02633..3fc45aed5bbb 100644 --- a/platform/util/src/com/intellij/util/containers/ContainerUtil.java +++ b/platform/util/src/com/intellij/util/containers/ContainerUtil.java @@ -2535,6 +2535,25 @@ public final class ContainerUtil { return -1; } + /** + * Finds the first element in the array that satisfies given condition. + * + * @param list array to scan + * @param condition condition that should be satisfied + * @param type of the list elements + * @return index of the first element in the array that satisfies the condition; -1 if no element satisfies the condition. + */ + @Contract(pure=true) + public static int indexOf(T @NotNull [] list, @NotNull Condition condition) { + for (int i = 0; i < list.length; i++) { + T t = list[i]; + if (condition.value(t)) { + return i; + } + } + return -1; + } + @Contract(pure=true) public static int lastIndexOf(@NotNull List list, @NotNull Condition condition) { for (int i = list.size() - 1; i >= 0; i--) { diff --git a/uast/uast-common-ide/src/com/intellij/psi/UastReferenceByUsageAdapter.kt b/uast/uast-common-ide/src/com/intellij/psi/UastReferenceByUsageAdapter.kt index 0bcd1e864c80..5abf9e0c785a 100644 --- a/uast/uast-common-ide/src/com/intellij/psi/UastReferenceByUsageAdapter.kt +++ b/uast/uast-common-ide/src/com/intellij/psi/UastReferenceByUsageAdapter.kt @@ -86,14 +86,11 @@ fun getReferencesForDirectUsagesOfVariable(element: PsiElement, targetHint: PsiE val uParentVariable = getUsageVariableTargetForInitializer(uElement) ?: return PsiReference.EMPTY_ARRAY val registrar = ReferenceProvidersRegistry.getInstance().getRegistrar(Language.findLanguageByID("UAST")!!) - val providerInfos = (registrar as PsiReferenceRegistrarImpl).getPairsByElement(originalElement, - PsiReferenceService.Hints(targetHint, null)) + val providers = (registrar as PsiReferenceRegistrarImpl).getPsiReferenceProvidersByElement(originalElement, + PsiReferenceService.Hints(targetHint, null)) // by-usage providers must implement acceptsTarget correctly, we rely on fact that they accept targetHint - val suitableProviders = providerInfos.asSequence() - .map { it.provider } - .filterIsInstance() - .toList() + val suitableProviders = providers.filterIsInstance() val usages = getDirectVariableUsagesWithNonLocal(uParentVariable) for (usage in usages) {