diff --git a/python/psi-api/src/com/jetbrains/python/psi/types/TypeEvalContextBasedCache.java b/python/psi-api/src/com/jetbrains/python/psi/types/TypeEvalContextBasedCache.java index 97057855c032..d02fb61eeee4 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/types/TypeEvalContextBasedCache.java +++ b/python/psi-api/src/com/jetbrains/python/psi/types/TypeEvalContextBasedCache.java @@ -15,13 +15,15 @@ */ package com.jetbrains.python.psi.types; -import com.intellij.psi.util.*; +import com.intellij.psi.util.CachedValue; +import com.intellij.psi.util.CachedValueProvider; +import com.intellij.psi.util.CachedValuesManager; +import com.intellij.psi.util.PsiModificationTracker; import com.intellij.util.Function; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; -import java.util.HashMap; -import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; /** * Engine to cache something in map, where {@link TypeEvalContext} is used as key. @@ -30,13 +32,8 @@ import java.util.Map; * @author Ilya.Kazakevich */ public final class TypeEvalContextBasedCache { - /** - * Lock to sync - */ @NotNull - private final Object myLock = new Object(); - @NotNull - private final CachedValue> myCachedMapStorage; + private final CachedValue> myCachedMapStorage; @NotNull private final Function myProvider; @@ -59,30 +56,24 @@ public final class TypeEvalContextBasedCache { @NotNull public T getValue(@NotNull final TypeEvalContext context) { - // Map is not thread safe, and "getValue" is not atomic. I do not want several maps to be created. - synchronized (myLock) { - final Map map = myCachedMapStorage.getValue(); - T value = map.get(context.getConstraints()); - if (value != null) { - return value; - } - // This is the same value, semantically: value for context-key - //noinspection ReuseOfLocalVariable - value = myProvider.fun(context); - map.put(context.getConstraints(), value); - return value; - } + // map is thread safe but not atomic nor getValue() is, so in worst case several threads may produce same result + // but explicit synchronization is also slow and may lead to deadlocks like in PY-24300 + final ConcurrentMap map = myCachedMapStorage.getValue(); + final TypeEvalConstraints key = context.getConstraints(); + map.computeIfAbsent(key, o -> myProvider.fun(context)); + return map.get(key); } /** * Provider that creates map to store cache. Map depends on PSI modification */ - private static final class MapCreator implements CachedValueProvider> { - @Nullable + private static final class MapCreator implements CachedValueProvider> { + @NotNull @Override - public Result> compute() { + public Result> compute() { // This method is called if cache is empty. Create new map for it. - final HashMap map = new HashMap<>(); + // Concurrent map allows several threads to call get and put, so it is thread safe but not atomic + final ConcurrentHashMap map = new ConcurrentHashMap<>(); return new Result<>(map, PsiModificationTracker.MODIFICATION_COUNT); } }