From dc9fa6d9ac9dab119bd5e112ecac9aa6ddf773f8 Mon Sep 17 00:00:00 2001 From: Nikolay Krasko Date: Thu, 21 Apr 2016 16:41:44 +0300 Subject: [PATCH] Don't compute cached values under lock - this can cause a deadlock Three threads needed to reproduce: - Thread 1. Doesn't hold RW lock, starts computation and locks the cached lock (L) - Thread 2. Holds R lock and want to get same computation result. It's now locked on (L) - Thread 3. Any thread that want to get W lock. It's now locked on because of Thread 2 If Thread 1. wants to get R lock it will be stacked because of Thread 3. It's sometimes not possible to get a read lock while some thread is already waiting on W. --- .../debugger/evaluate/KotlinDebuggerCaches.kt | 103 ++++++++++-------- 1 file changed, 57 insertions(+), 46 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt b/idea/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt index 5575921d168..ed4b5002636 100644 --- a/idea/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt +++ b/idea/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt @@ -43,6 +43,7 @@ import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.types.KotlinType import java.util.* +import java.util.concurrent.ConcurrentHashMap class KotlinDebuggerCaches(private val project: Project) { @@ -54,14 +55,16 @@ class KotlinDebuggerCaches(private val project: Project) { private val cachedClassNames = CachedValuesManager.getManager(project).createCachedValue( { - CachedValueProvider.Result>>( - hashMapOf(), PsiModificationTracker.MODIFICATION_COUNT) + CachedValueProvider.Result>>( + ConcurrentHashMap>(), + PsiModificationTracker.MODIFICATION_COUNT) }, false) private val cachedTypeMappers = CachedValuesManager.getManager(project).createCachedValue( { - CachedValueProvider.Result>( - hashMapOf(), PsiModificationTracker.MODIFICATION_COUNT) + CachedValueProvider.Result>( + ConcurrentHashMap(), + PsiModificationTracker.MODIFICATION_COUNT) }, false) companion object { @@ -77,67 +80,75 @@ class KotlinDebuggerCaches(private val project: Project) { ): CompiledDataDescriptor { val evaluateExpressionCache = getInstance(codeFragment.project) - return synchronized(evaluateExpressionCache.cachedCompiledData) { + val text = "${codeFragment.importsToString()}\n${codeFragment.text}" + + val cached = synchronized>(evaluateExpressionCache.cachedCompiledData) { val cache = evaluateExpressionCache.cachedCompiledData.value!! - val text = "${codeFragment.importsToString()}\n${codeFragment.text}" - val answer = cache[text].firstOrNull { - it.sourcePosition == sourcePosition || evaluateExpressionCache.canBeEvaluatedInThisContext(it, evaluationContext) - } - if (answer != null) return@synchronized answer - - val newCompiledData = create(codeFragment, sourcePosition) - LOG.debug("Compile bytecode for ${codeFragment.text}") - - cache.putValue(text, newCompiledData) - return@synchronized newCompiledData + cache[text] } + + val answer = cached.firstOrNull { + it.sourcePosition == sourcePosition || evaluateExpressionCache.canBeEvaluatedInThisContext(it, evaluationContext) + } + if (answer != null) { + return answer + } + + val newCompiledData = create(codeFragment, sourcePosition) + LOG.debug("Compile bytecode for ${codeFragment.text}") + + synchronized(evaluateExpressionCache.cachedCompiledData) { + evaluateExpressionCache.cachedCompiledData.value.putValue(text, newCompiledData) + } + + return newCompiledData } fun getOrComputeClassNames(psiElement: T, create: (T) -> ComputedClassNames): List { val cache = getInstance(runReadAction { psiElement.project }) - synchronized(cache.cachedClassNames) { - val classNamesCache = cache.cachedClassNames.value - val cachedValue = classNamesCache[psiElement] - if (cachedValue != null) return cachedValue + val classNamesCache = cache.cachedClassNames.value - val computedClassNames = create(psiElement) + val cachedValue = classNamesCache[psiElement] + if (cachedValue != null) return cachedValue - if (computedClassNames.shouldBeCached) { - classNamesCache[psiElement] = computedClassNames.classNames - } - return computedClassNames.classNames + val computedClassNames = create(psiElement) + + if (computedClassNames.shouldBeCached) { + classNamesCache[psiElement] = computedClassNames.classNames } + + return computedClassNames.classNames } fun getOrCreateTypeMapper(psiElement: PsiElement): KotlinTypeMapper { val cache = getInstance(runReadAction { psiElement.project }) - synchronized(cache.cachedTypeMappers) { - val typeMappersCache = cache.cachedTypeMappers.value - val file = runReadAction { psiElement.containingFile as KtFile } - val isInLibrary = LibraryUtil.findLibraryEntry(file.virtualFile, file.project) != null + val file = runReadAction { psiElement.containingFile as KtFile } + val isInLibrary = LibraryUtil.findLibraryEntry(file.virtualFile, file.project) != null - if (!isInLibrary) { - // Key = file - val cachedValue = typeMappersCache[file] - if (cachedValue != null) return cachedValue + val key = if (!isInLibrary) file else psiElement - val newValue = createTypeMapperForSourceFile(file) - typeMappersCache[file] = newValue - return newValue - } - else { - // key = KtElement - val element = getElementToCreateTypeMapperForLibraryFile(psiElement) - val cachedValue = typeMappersCache[psiElement] - if (cachedValue != null) return cachedValue + val typeMappersCache = cache.cachedTypeMappers.value - val newValue = createTypeMapperForLibraryFile(element, file) - typeMappersCache[psiElement] = newValue - return newValue - } + val cachedValue = typeMappersCache[key] + if (cachedValue != null) return cachedValue + + if (!isInLibrary) { + val newValue = createTypeMapperForSourceFile(file) + + typeMappersCache[file] = newValue + + return newValue + } + else { + val element = getElementToCreateTypeMapperForLibraryFile(psiElement) + val newValue = createTypeMapperForLibraryFile(element, file) + + typeMappersCache[psiElement] = newValue + + return newValue } }