From 2d82cb936e6cb824ce812a0cb3d9f3e9162ccb14 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Wed, 15 Nov 2017 11:24:08 +0300 Subject: [PATCH] Fix potential memory leak in light-classes The problem is that they might indirectly retain a reference to an obsolete ResolverForProject (it becomes outdated after out of block modification). The best solution would be to avoid preserving jvm-related diagnostics (that are only used once to report them when highlighting, and exactly they retains an obsolete resolve session) But it seems that this may lead to a deep light-classes refactoring and the temporary fix is storing diagnostics themselves in a CachedValue bound to out of block modification tracker. There're no guarantees that this would help, since CachedValue anyway is based on soft-reference that is not the first thing GC collects --- .../resolve/IDELightClassGenerationSupport.kt | 10 ++++- .../lightClasses/LazyLightClassDataHolder.kt | 41 +++++++++++++------ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IDELightClassGenerationSupport.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IDELightClassGenerationSupport.kt index 96e7b7de493..c77895c6e36 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IDELightClassGenerationSupport.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/IDELightClassGenerationSupport.kt @@ -55,15 +55,19 @@ class IDELightClassGenerationSupport(private val project: Project) : LightClassG return if (classOrObject.isLocal) { LazyLightClassDataHolder.ForClass( builder, + classOrObject.project, exactContextProvider = { IDELightClassContexts.contextForLocalClassOrObject(classOrObject) }, - dummyContextProvider = null + dummyContextProvider = null, + isLocal = true ) } else { LazyLightClassDataHolder.ForClass( builder, + classOrObject.project, exactContextProvider = { IDELightClassContexts.contextForNonLocalClassOrObject(classOrObject) }, - dummyContextProvider = { IDELightClassContexts.lightContextForClassOrObject(classOrObject) } + dummyContextProvider = { IDELightClassContexts.lightContextForClassOrObject(classOrObject) }, + isLocal = false ) } } @@ -76,6 +80,7 @@ class IDELightClassGenerationSupport(private val project: Project) : LightClassG return LazyLightClassDataHolder.ForFacade( builder, + files.first().project, exactContextProvider = { IDELightClassContexts.contextForFacade(sortedFiles) }, dummyContextProvider = { IDELightClassContexts.lightContextForFacade(sortedFiles) } ) @@ -84,6 +89,7 @@ class IDELightClassGenerationSupport(private val project: Project) : LightClassG override fun createDataHolderForScript(script: KtScript, builder: LightClassBuilder): LightClassDataHolder.ForScript { return LazyLightClassDataHolder.ForScript( builder, + script.project, exactContextProvider = { IDELightClassContexts.contextForScript(script) }, dummyContextProvider = { IDELightClassContexts.lightContextForScript(script) } ) diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/lightClasses/LazyLightClassDataHolder.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/lightClasses/LazyLightClassDataHolder.kt index 8008da7c3e9..caffceb413a 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/lightClasses/LazyLightClassDataHolder.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/lightClasses/LazyLightClassDataHolder.kt @@ -16,11 +16,15 @@ package org.jetbrains.kotlin.idea.caches.resolve.lightClasses +import com.intellij.openapi.project.Project import com.intellij.openapi.util.Key import com.intellij.psi.PsiClass import com.intellij.psi.PsiMember import com.intellij.psi.PsiMethod import com.intellij.psi.impl.java.stubs.PsiJavaFileStub +import com.intellij.psi.util.CachedValueProvider +import com.intellij.psi.util.CachedValuesManager +import com.intellij.psi.util.PsiModificationTracker import org.jetbrains.kotlin.asJava.LightClassBuilder import org.jetbrains.kotlin.asJava.builder.* import org.jetbrains.kotlin.asJava.classes.KtLightClass @@ -37,23 +41,31 @@ typealias DummyLightClassContextProvider = (() -> LightClassConstructionContext? sealed class LazyLightClassDataHolder( builder: LightClassBuilder, + project: Project, exactContextProvider: ExactLightClassContextProvider, - dummyContextProvider: DummyLightClassContextProvider + dummyContextProvider: DummyLightClassContextProvider, + isLocal: Boolean = false ) : LightClassDataHolder { - private val exactResultLazyValue = lazyPub { builder(exactContextProvider()) } - - private val exactResult: LightClassBuilderResult by exactResultLazyValue + private val exactResultCachedValue = + CachedValuesManager.getManager(project).createCachedValue({ + CachedValueProvider.Result.create( + builder(exactContextProvider()), + if (isLocal) + PsiModificationTracker.MODIFICATION_COUNT + else + PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT) + }, false) private val lazyInexactStub by lazyPub { dummyContextProvider?.let { provider -> provider()?.let { context -> builder.invoke(context).stub } } } private val inexactStub: PsiJavaFileStub? - get() = if (exactResultLazyValue.isInitialized()) null else lazyInexactStub + get() = if (exactResultCachedValue.hasUpToDateValue()) null else lazyInexactStub - override val javaFileStub get() = exactResult.stub - override val extraDiagnostics get() = exactResult.diagnostics + override val javaFileStub get() = exactResultCachedValue.value.stub + override val extraDiagnostics get() = exactResultCachedValue.value.diagnostics // for facade or defaultImpls override fun findData(findDelegate: (PsiJavaFileStub) -> PsiClass): LightClassData = @@ -62,8 +74,9 @@ sealed class LazyLightClassDataHolder( } class ForClass( - builder: LightClassBuilder, exactContextProvider: ExactLightClassContextProvider, dummyContextProvider: DummyLightClassContextProvider - ) : LazyLightClassDataHolder(builder, exactContextProvider, dummyContextProvider), LightClassDataHolder.ForClass { + builder: LightClassBuilder, project: Project, isLocal: Boolean, + exactContextProvider: ExactLightClassContextProvider, dummyContextProvider: DummyLightClassContextProvider + ) : LazyLightClassDataHolder(builder, project, exactContextProvider, dummyContextProvider, isLocal), LightClassDataHolder.ForClass { override fun findDataForClassOrObject(classOrObject: KtClassOrObject): LightClassData = LazyLightClassData { stub -> stub.findDelegate(classOrObject) @@ -71,12 +84,14 @@ sealed class LazyLightClassDataHolder( } class ForFacade( - builder: LightClassBuilder, exactContextProvider: ExactLightClassContextProvider, dummyContextProvider: DummyLightClassContextProvider - ) : LazyLightClassDataHolder(builder, exactContextProvider, dummyContextProvider), LightClassDataHolder.ForFacade + builder: LightClassBuilder, project: Project, + exactContextProvider: ExactLightClassContextProvider, dummyContextProvider: DummyLightClassContextProvider + ) : LazyLightClassDataHolder(builder, project, exactContextProvider, dummyContextProvider), LightClassDataHolder.ForFacade class ForScript( - builder: LightClassBuilder, exactContextProvider: ExactLightClassContextProvider, dummyContextProvider: DummyLightClassContextProvider - ) : LazyLightClassDataHolder(builder, exactContextProvider, dummyContextProvider), LightClassDataHolder.ForScript + builder: LightClassBuilder, project: Project, + exactContextProvider: ExactLightClassContextProvider, dummyContextProvider: DummyLightClassContextProvider + ) : LazyLightClassDataHolder(builder, project, exactContextProvider, dummyContextProvider), LightClassDataHolder.ForScript private inner class LazyLightClassData( findDelegate: (PsiJavaFileStub) -> PsiClass