From bef33e971ff69ff17442529f597ae104f643cb6b Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Mon, 23 Sep 2019 18:47:37 +0300 Subject: [PATCH] Make ResolverForProject own BuiltInsCache Previously, BuiltInsCache was a separate abstraction with its own lifecycle. In particular, it had a CachedValue inside, which uses SoftReference to hold the computed data. This is bad, because this might lead to disalignment of lifetimes with respective ResolverForProject, leading to potential exceptions in analysis, see KT-33504 for details. This commit fixes it by making root ResolverForProject own BuiltInsCache and removing any logic about invalidation from BuiltInsCache. So, now, BuiltInsCache is disposed iff corresponding ResolverForProject is disposed ^KT-33504 Fixed --- .../caches/resolve/IdeaResolverForProject.kt | 44 +++++++++++++++---- .../caches/resolve/KotlinCacheServiceImpl.kt | 44 +++---------------- .../caches/resolve/ProjectResolutionFacade.kt | 30 +------------ 3 files changed, 42 insertions(+), 76 deletions(-) diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IdeaResolverForProject.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IdeaResolverForProject.kt index cd1108693c4..02dae25d47a 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IdeaResolverForProject.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IdeaResolverForProject.kt @@ -9,7 +9,6 @@ import com.intellij.openapi.components.ServiceManager import com.intellij.openapi.util.ModificationTracker import org.jetbrains.kotlin.analyzer.* import org.jetbrains.kotlin.analyzer.common.CommonAnalysisParameters -import org.jetbrains.kotlin.builtins.DefaultBuiltIns import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.builtins.jvm.JvmBuiltIns import org.jetbrains.kotlin.caches.resolve.CompositeAnalyzerServices @@ -40,10 +39,9 @@ import org.jetbrains.kotlin.resolve.jvm.JvmPlatformParameters class IdeaResolverForProject( debugName: String, projectContext: ProjectContext, - private val modules: Collection, + modules: Collection, private val syntheticFilesByModule: Map>, delegateResolver: ResolverForProject, - private val builtInsCache: BuiltInsCache, fallbackModificationTracker: ModificationTracker? = null, private val isReleaseCoroutines: Boolean? = null, // TODO(dsavvinov): this is needed only for non-composite analysis, extract separate resolver implementation instead @@ -56,6 +54,8 @@ class IdeaResolverForProject( delegateResolver, ServiceManager.getService(projectContext.project, IdePackageOracleFactory::class.java) ) { + private val builtInsCache: BuiltInsCache = (delegateResolver as? IdeaResolverForProject)?.builtInsCache ?: BuiltInsCache(projectContext, this) + override fun sdkDependency(module: IdeaModuleInfo): IdeaModuleInfo? { if (projectContext.project.useCompositeAnalysis) { require(constantSdkDependencyIfAny == null) { "Shouldn't pass SDK dependency manually for composite analysis mode" } @@ -66,11 +66,7 @@ class IdeaResolverForProject( override fun modulesContent(module: IdeaModuleInfo): ModuleContent = ModuleContent(module, syntheticFilesByModule[module] ?: emptyList(), module.contentScope()) - override fun builtInsForModule(module: IdeaModuleInfo): KotlinBuiltIns { - require(module in modules) - val key = module.platform.idePlatformKind.resolution.getKeyForBuiltIns(module) - return builtInsCache.getOrPut(key) { throw IllegalStateException("Can't find builtIns by key $key for module $module") } - } + override fun builtInsForModule(module: IdeaModuleInfo): KotlinBuiltIns = builtInsCache.getOrCreateIfNeeded(module) override fun createResolverForModule(descriptor: ModuleDescriptor, moduleInfo: IdeaModuleInfo): ResolverForModule { val moduleContent = ModuleContent(moduleInfo, syntheticFilesByModule[moduleInfo] ?: listOf(), moduleInfo.contentScope()) @@ -121,4 +117,36 @@ class IdeaResolverForProject( ) } } + + // Important: ProjectContext must be from SDK to be sure that we won't run into deadlocks + class BuiltInsCache(private val projectContextFromSdkResolver: ProjectContext, private val resolverForSdk: IdeaResolverForProject) { + private val cache = mutableMapOf() + + fun getOrCreateIfNeeded(module: IdeaModuleInfo): KotlinBuiltIns = projectContextFromSdkResolver.storageManager.compute { + val key = module.platform.idePlatformKind.resolution.getKeyForBuiltIns(module) + val cachedBuiltIns = cache[key] + if (cachedBuiltIns != null) return@compute cachedBuiltIns + + // Note #1: we can't use .getOrPut, because we have to put builtIns into map *before* initialization + // Note #2: it's OK to put not-initialized built-ins into public map, because access to [cache] is guarded by storageManager.lock + val newBuiltIns = module.platform.idePlatformKind.resolution.createBuiltIns(module, projectContextFromSdkResolver) + cache[key] = newBuiltIns + + if (newBuiltIns is JvmBuiltIns) { + // SDK should be present, otherwise we wouldn't have created JvmBuiltIns in createBuiltIns + val sdk = module.findSdkAcrossDependencies()!! + val sdkDescriptor = resolverForSdk.descriptorForModule(sdk) + + val isAdditionalBuiltInsFeaturesSupported = module.supportsAdditionalBuiltInsMembers(projectContextFromSdkResolver.project) + + newBuiltIns.initialize(sdkDescriptor, isAdditionalBuiltInsFeaturesSupported) + } + + return@compute newBuiltIns + } + } +} + +interface BuiltInsCacheKey { + object DefaultBuiltInsKey : BuiltInsCacheKey } \ No newline at end of file diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt index 94da09d4684..36ca5ea187d 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt @@ -177,8 +177,7 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { //TODO: provide correct trackers dependencies = dependenciesForScriptDependencies, moduleFilter = { it == dependenciesModuleInfo }, - invalidateOnOOCB = true, - builtInsCache = globalFacade.builtInsCache + invalidateOnOOCB = true ) } @@ -194,8 +193,7 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { ProjectRootModificationTracker.getInstance(project) ), invalidateOnOOCB = false, - reuseDataFrom = null, - builtInsCache = BuiltInsCache(project) + reuseDataFrom = null ) private val librariesContext = sdkContext.contextWithCompositeExceptionTracker(project, resolverForLibrariesName) @@ -208,8 +206,7 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { dependencies = listOf( LibraryModificationTracker.getInstance(project), ProjectRootModificationTracker.getInstance(project) - ), - builtInsCache = facadeForSdk.builtInsCache + ) ) private val modulesContext = librariesContext.contextWithCompositeExceptionTracker(project, resolverForModulesName) @@ -222,8 +219,7 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { LibraryModificationTracker.getInstance(project), ProjectRootModificationTracker.getInstance(project) ), - invalidateOnOOCB = true, - builtInsCache = facadeForLibraries.builtInsCache + invalidateOnOOCB = true ) } @@ -294,8 +290,7 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { moduleFilter = moduleFilter, dependencies = dependenciesForSyntheticFileCache, invalidateOnOOCB = true, - allModules = allModules, - builtInsCache = reuseDataFrom?.builtInsCache ?: BuiltInsCache(project) + allModules = allModules ) } @@ -538,35 +533,6 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { } } -class BuiltInsCache(private val project: Project) { - private val innerCache - get() = project.cacheByClass( - BuiltInsCache::class.java, - ProjectRootModificationTracker.getInstance(project), - LibraryModificationTracker.getInstance(project) - ) { ConcurrentHashMap() } - - init { - innerCache[BuiltInsCacheKey.DefaultBuiltInsKey] = DefaultBuiltIns.Instance - } - - fun getOrPut(key: BuiltInsCacheKey, ifAbsent: (BuiltInsCacheKey) -> KotlinBuiltIns): KotlinBuiltIns { - return innerCache.computeIfAbsent(key, ifAbsent) - } - - operator fun get(key: BuiltInsCacheKey): KotlinBuiltIns? { - return innerCache[key] - } - - operator fun set(key: BuiltInsCacheKey, value: KotlinBuiltIns) { - innerCache[key] = value - } -} - -interface BuiltInsCacheKey { - object DefaultBuiltInsKey : BuiltInsCacheKey -} - fun IdeaModuleInfo.supportsAdditionalBuiltInsMembers(project: Project): Boolean { return IDELanguageSettingsProvider .getLanguageVersionSettings(this, project) diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/ProjectResolutionFacade.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/ProjectResolutionFacade.kt index b0b3420a01a..1911924dbae 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/ProjectResolutionFacade.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/ProjectResolutionFacade.kt @@ -23,14 +23,11 @@ import com.intellij.psi.util.CachedValuesManager import com.intellij.psi.util.PsiModificationTracker import com.intellij.util.containers.SLRUCache import org.jetbrains.kotlin.analyzer.* -import org.jetbrains.kotlin.builtins.jvm.JvmBuiltIns -import org.jetbrains.kotlin.caches.resolve.resolution import org.jetbrains.kotlin.context.GlobalContextImpl import org.jetbrains.kotlin.context.withProject import org.jetbrains.kotlin.descriptors.ModuleDescriptor import org.jetbrains.kotlin.idea.caches.project.* import org.jetbrains.kotlin.idea.caches.project.IdeaModuleInfo -import org.jetbrains.kotlin.platform.idePlatformKind import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.resolve.CompositeBindingContext @@ -46,7 +43,6 @@ internal class ProjectResolutionFacade( val moduleFilter: (IdeaModuleInfo) -> Boolean, dependencies: List, private val invalidateOnOOCB: Boolean, - val builtInsCache: BuiltInsCache, val syntheticFiles: Collection = listOf(), val allModules: Collection? = null // null means create resolvers for modules from idea model ) { @@ -88,7 +84,6 @@ internal class ProjectResolutionFacade( } else { delegateResolverForProject = EmptyResolverForProject() } - val projectContext = globalContext.withProject(project) val allModuleInfos = (allModules ?: getModuleInfosFromIdeaModel(project, (settings as? PlatformAnalysisSettingsImpl)?.platform)) .toMutableSet() @@ -101,38 +96,15 @@ internal class ProjectResolutionFacade( val resolverForProject = IdeaResolverForProject( resolverDebugName, - projectContext, + globalContext.withProject(project), modulesToCreateResolversFor, syntheticFilesByModule, delegateResolverForProject, - builtInsCache, if (invalidateOnOOCB) KotlinModificationTrackerService.getInstance(project).outOfBlockModificationTracker else null, settings.isReleaseCoroutines, constantSdkDependencyIfAny = if (settings is PlatformAnalysisSettingsImpl) settings.sdk?.let { SdkInfo(project, it) } else null ) - // Fill builtInsCache - modulesToCreateResolversFor.forEach { - val key = it.platform.idePlatformKind.resolution.getKeyForBuiltIns(it) - val cachedBuiltIns = builtInsCache[key] - if (cachedBuiltIns == null) { - // Note that we can't use .getOrPut, because we have to put builtIns into map *before* - // initialization - val builtIns = it.platform.idePlatformKind.resolution.createBuiltIns(it, projectContext) - builtInsCache[key] = builtIns - - if (builtIns is JvmBuiltIns) { - // SDK should be present, otherwise we wouldn't have created JvmBuiltIns in createBuiltIns - val sdk = it.findSdkAcrossDependencies()!! - val sdkDescriptor = resolverForProject.descriptorForModule(sdk) - - val isAdditionalBuiltInsFeaturesSupported = it.supportsAdditionalBuiltInsMembers(project) - - builtIns.initialize(sdkDescriptor, isAdditionalBuiltInsFeaturesSupported) - } - } - } - return resolverForProject }