From 25dc81c2bd7fb01cf4c9ed88b7f37b370d015952 Mon Sep 17 00:00:00 2001 From: Andrey Uskov Date: Mon, 18 Feb 2019 20:02:23 +0300 Subject: [PATCH] Fix memory leak in gradle import #KT-30076 Fixed --- .../KotlinGradleProjectResolverExtension.kt | 15 ++- .../KotlinMPPGradleProjectResolver.kt | 16 +-- .../ExternalSystemImportingTestCase.java | 100 +++++++++++++----- .../src/KotlinGradleModelBuilder.kt | 22 +++- .../src/KotlinMPPGradleModel.kt | 5 +- .../src/KotlinMPPGradleModelBuilder.kt | 5 +- .../src/KotlinMPPGradleModelBuilder.kt.181 | 5 +- .../src/KotlinMPPGradleModelImpl.kt | 72 ++++++++++--- 8 files changed, 174 insertions(+), 66 deletions(-) diff --git a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinGradleProjectResolverExtension.kt b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinGradleProjectResolverExtension.kt index 0bf91dbc472..0c04c130ed3 100644 --- a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinGradleProjectResolverExtension.kt +++ b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinGradleProjectResolverExtension.kt @@ -29,10 +29,7 @@ import com.intellij.openapi.util.Key import com.intellij.openapi.util.io.FileUtil import org.gradle.api.artifacts.Dependency import org.gradle.tooling.model.idea.IdeaModule -import org.jetbrains.kotlin.gradle.CompilerArgumentsBySourceSet -import org.jetbrains.kotlin.gradle.KotlinGradleModel -import org.jetbrains.kotlin.gradle.KotlinGradleModelBuilder -import org.jetbrains.kotlin.gradle.KotlinMPPGradleModel +import org.jetbrains.kotlin.gradle.* import org.jetbrains.kotlin.idea.inspections.gradle.getDependencyModules import org.jetbrains.kotlin.idea.util.CopyableDataNodeUserDataProperty import org.jetbrains.kotlin.idea.util.DataNodeUserDataProperty @@ -203,8 +200,8 @@ class KotlinGradleProjectResolverExtension : AbstractProjectResolverExtension() if (mppModel != null) { mppModel.targets.forEach { target -> KotlinStatisticsTrigger.trigger( - KotlinEventTrigger.KotlinGradleTargetTrigger, - "MPP.${target.platform.id + (target.presetName?.let { ".$it"} ?: "")}" + KotlinEventTrigger.KotlinGradleTargetTrigger, + "MPP.${target.platform.id + (target.presetName?.let { ".$it" } ?: "")}" ) } return super.populateModuleDependencies(gradleModule, ideModule, ideProject) @@ -222,13 +219,13 @@ class KotlinGradleProjectResolverExtension : AbstractProjectResolverExtension() ideModule.isResolved = true ideModule.hasKotlinPlugin = gradleModel.hasKotlinPlugin - ideModule.compilerArgumentsBySourceSet = gradleModel.compilerArgumentsBySourceSet + ideModule.compilerArgumentsBySourceSet = gradleModel.compilerArgumentsBySourceSet.deepCopy() ideModule.coroutines = gradleModel.coroutines ideModule.platformPluginId = gradleModel.platformPluginId KotlinStatisticsTrigger.trigger( - KotlinEventTrigger.KotlinGradleTargetTrigger, - gradleModel.kotlinTarget ?: "unknown" + KotlinEventTrigger.KotlinGradleTargetTrigger, + gradleModel.kotlinTarget ?: "unknown" ) addImplementedModuleNames(gradleModule, ideModule, ideProject, gradleModel) diff --git a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt index 48be8fbc662..276b1dc43b5 100644 --- a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt +++ b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt @@ -613,7 +613,7 @@ open class KotlinMPPGradleProjectResolver : AbstractProjectResolverExtension() { resolverCtx: ProjectResolverContext ): KotlinSourceSetInfo? { if (sourceSet.platform.isNotSupported()) return null - return KotlinSourceSetInfo(sourceSet).also { info -> + return KotlinSourceSetInfo(KotlinSourceSetImpl(sourceSet)).also { info -> val languageSettings = sourceSet.languageSettings info.moduleId = getKotlinModuleId(gradleModule, sourceSet, resolverCtx) info.gradleModuleId = getModuleId(resolverCtx, gradleModule) @@ -643,16 +643,18 @@ open class KotlinMPPGradleProjectResolver : AbstractProjectResolverExtension() { resolverCtx: ProjectResolverContext ): KotlinSourceSetInfo? { if (compilation.platform.isNotSupported()) return null - return KotlinSourceSetInfo(compilation).also { sourceSetInfo -> + return KotlinSourceSetInfo(KotlinCompilationImpl(compilation)).also { sourceSetInfo -> sourceSetInfo.moduleId = getKotlinModuleId(gradleModule, compilation, resolverCtx) sourceSetInfo.gradleModuleId = getModuleId(resolverCtx, gradleModule) sourceSetInfo.platform = compilation.platform sourceSetInfo.isTestModule = compilation.isTestModule - sourceSetInfo.compilerArguments = createCompilerArguments(compilation.arguments.currentArguments, compilation.platform).also { - it.multiPlatform = true - } + sourceSetInfo.compilerArguments = + createCompilerArguments(compilation.arguments.currentArguments, compilation.platform).also { + it.multiPlatform = true + } sourceSetInfo.dependencyClasspath = compilation.dependencyClasspath - sourceSetInfo.defaultCompilerArguments = createCompilerArguments(compilation.arguments.defaultArguments, compilation.platform) + sourceSetInfo.defaultCompilerArguments = + createCompilerArguments(compilation.arguments.defaultArguments, compilation.platform) sourceSetInfo.addSourceSets(compilation.sourceSets, compilation.fullName(), gradleModule, resolverCtx) } } @@ -679,7 +681,7 @@ open class KotlinMPPGradleProjectResolver : AbstractProjectResolverExtension() { } private fun KotlinModule.fullName(simpleName: String = name) = when (this) { - is KotlinCompilation -> compilationFullName(simpleName, target.disambiguationClassifier) + is KotlinCompilation -> compilationFullName(simpleName, disambiguationClassifier) else -> simpleName } diff --git a/idea/idea-gradle/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/ExternalSystemImportingTestCase.java b/idea/idea-gradle/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/ExternalSystemImportingTestCase.java index ed087bc3144..ecaccdddd06 100644 --- a/idea/idea-gradle/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/ExternalSystemImportingTestCase.java +++ b/idea/idea-gradle/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/ExternalSystemImportingTestCase.java @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.codeInsight.gradle; import com.intellij.openapi.application.AccessToken; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.components.ServiceManager; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.externalSystem.importing.ImportSpecBuilder; import com.intellij.openapi.externalSystem.model.DataNode; import com.intellij.openapi.externalSystem.model.ProjectSystemId; @@ -49,9 +50,13 @@ import java.lang.reflect.Array; import java.lang.reflect.Field; import java.lang.reflect.Proxy; import java.util.*; +import org.apache.log4j.Level; // part of com.intellij.openapi.externalSystem.test.ExternalSystemImportingTestCase public abstract class ExternalSystemImportingTestCase extends ExternalSystemTestCase { + + private Logger logger; + @Override protected Module getModule(String name) { AccessToken accessToken = ApplicationManager.getApplication().acquireReadActionLock(); @@ -74,16 +79,26 @@ public abstract class ExternalSystemImportingTestCase extends ExternalSystemTest doImportProject(); } - public void ensureIsNotGradleProxyObject(Object o, Map referencingObjects) { - if (o instanceof Proxy) { - StringBuilder errMessage = new StringBuilder(); - errMessage.append(String.format("Object [%s] seems to be a referenced gradle tooling api object. (it may lead to memory leaks during import) Referencing path: ", o)); - while (o != null) { - errMessage.append(String.format("[%s] type: %s <-\r\n", o, o.getClass().toString())); - o = referencingObjects.get(o); - } - //TODO fail(errMessage.toString()); + public boolean ensureIsNotGradleProxyObject( + Object o, + Map referencingObjects, + Map referencingFieldNames + ) { + if (!(o instanceof Proxy)) { + return true; } + StringBuilder errMessage = new StringBuilder(); + errMessage.append(String.format( + "Object [%s] seems to be a referenced gradle tooling api object. (it may lead to memory leaks during import) Referencing path: ", + o)); + while (o != null) { + errMessage.append(String.format("[%s].[%s] type: %s <-\r\n", o, referencingFieldNames.get(o), o.getClass().toString())); + o = referencingObjects.get(o); + } + System.err.println(errMessage.toString()); + //TODO uncomment after fixing IDEA-207782 + //fail(errMessage.toString()); + return false; } private boolean shouldBeProcessed(Object toProcess, Set processed) { @@ -91,56 +106,83 @@ public abstract class ExternalSystemImportingTestCase extends ExternalSystemTest } public void inspectForGradleMemoryLeaks(DataNode externalProject) { + // Static logger initialisation should not be used because logger factory may be changed on test.setUp + if (logger == null) { + logger = Logger.getInstance(this.getClass().getName()); + logger.setLevel(Level.INFO);//not required + } long start = System.currentTimeMillis(); Set processed = new HashSet<>(); Queue toProcess = new LinkedList<>(); Map referencingObjects = new HashMap<>(); + Map referencingFieldNames = new HashMap<>(); + Set modifiedFields = new HashSet<>(); toProcess.add(externalProject); try { - while (! toProcess.isEmpty()) { + while (!toProcess.isEmpty()) { Object nextObject = toProcess.poll(); processed.add(nextObject); - ensureIsNotGradleProxyObject(nextObject, referencingObjects); + if (!ensureIsNotGradleProxyObject(nextObject, referencingObjects, referencingFieldNames)) { + break; + } for (Field field : nextObject.getClass().getDeclaredFields()) { try { - final Object fieldValue; - boolean isAccessible = field.isAccessible(); - try { + + if (!field.isAccessible()) { field.setAccessible(true); - fieldValue = field.get(nextObject); - } finally { - field.setAccessible(isAccessible); + modifiedFields.add(field); } - if (fieldValue == null) { + final Object fieldValue = field.get(nextObject); + if (fieldValue == null || fieldValue.getClass().isPrimitive()) { continue; } if (fieldValue instanceof Collection) { - for (Object o : (Collection)fieldValue) { - saveToProcessIfRequired(processed, toProcess, referencingObjects, nextObject, o); + for (Object o : (Collection) fieldValue) { + saveToProcessIfRequired(processed, toProcess, referencingObjects, referencingFieldNames, nextObject, o, + field.getName()); } - } else if (fieldValue.getClass().isArray()) { + } + else if (fieldValue.getClass().isArray()) { for (int i = 0; i < Array.getLength(fieldValue); i++) { Object o = Array.get(fieldValue, i); - saveToProcessIfRequired(processed, toProcess, referencingObjects, nextObject, o); + saveToProcessIfRequired(processed, toProcess, referencingObjects, referencingFieldNames, nextObject, o, + field.getName()); } - } else { - saveToProcessIfRequired(processed, toProcess, referencingObjects, nextObject, fieldValue); } - } catch (IllegalAccessException e) { + else { + saveToProcessIfRequired(processed, toProcess, referencingObjects, referencingFieldNames, nextObject, fieldValue, + field.getName()); + } + } + catch (IllegalAccessException e) { fail(e.getMessage()); } } - } - } finally { - System.out.println("Processed size = " + processed.size() + " Duration: " + (System.currentTimeMillis() - start)); + } + finally { + for (Field f : modifiedFields) { + f.setAccessible(false); + } + logger.info( + String.format("Memory leak tracker has finished. Number of processed objects = %s. Duration = %s ms.", processed.size(), + (System.currentTimeMillis() - start))); } } - private void saveToProcessIfRequired(Set processed, Queue toProcess, Map referrers, Object referringObject, Object o) { + private void saveToProcessIfRequired( + Set processed, + Queue toProcess, + Map referrers, + Map referencingFieldNames, + Object referringObject, + Object o, + String fieldName + ) { if (shouldBeProcessed(o, processed)) { toProcess.add(o); + referencingFieldNames.put(o, fieldName); referrers.put(o, referringObject); } } diff --git a/idea/kotlin-gradle-tooling/src/KotlinGradleModelBuilder.kt b/idea/kotlin-gradle-tooling/src/KotlinGradleModelBuilder.kt index c7e07f80a24..85c092b9525 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinGradleModelBuilder.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinGradleModelBuilder.kt @@ -34,14 +34,30 @@ interface ArgsInfo : Serializable { val dependencyClasspath: List } -class ArgsInfoImpl( +data class ArgsInfoImpl( override val currentArguments: List, override val defaultArguments: List, override val dependencyClasspath: List -) : ArgsInfo +) : ArgsInfo { + + constructor(argsInfo: ArgsInfo) : this( + ArrayList(argsInfo.currentArguments), + ArrayList(argsInfo.defaultArguments), + ArrayList(argsInfo.dependencyClasspath) + ) +} typealias CompilerArgumentsBySourceSet = Map +/** + * Creates deep copy in order to avoid holding links to Proxy objects created by gradle tooling api + */ +fun CompilerArgumentsBySourceSet.deepCopy(): CompilerArgumentsBySourceSet { + val result = HashMap() + this.forEach { key, value -> result[key] = ArgsInfoImpl(value) } + return result +} + interface KotlinGradleModel : Serializable { val hasKotlinPlugin: Boolean val compilerArgumentsBySourceSet: CompilerArgumentsBySourceSet @@ -51,7 +67,7 @@ interface KotlinGradleModel : Serializable { val kotlinTarget: String? } -class KotlinGradleModelImpl( +data class KotlinGradleModelImpl( override val hasKotlinPlugin: Boolean, override val compilerArgumentsBySourceSet: CompilerArgumentsBySourceSet, override val coroutines: String?, diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt index 6018c230835..a30e998d1bd 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModel.kt @@ -6,11 +6,14 @@ package org.jetbrains.kotlin.gradle import org.jetbrains.plugins.gradle.model.ExternalDependency +import org.jetbrains.plugins.gradle.model.ModelFactory import java.io.File import java.io.Serializable typealias KotlinDependency = ExternalDependency +fun KotlinDependency.deepCopy(): KotlinDependency = ModelFactory.createCopy(this) + interface KotlinModule : Serializable { val name: String val platform: KotlinPlatform @@ -55,10 +58,10 @@ interface KotlinCompilationArguments : Serializable { interface KotlinCompilation : KotlinModule { val sourceSets: Collection - val target: KotlinTarget val output: KotlinCompilationOutput val arguments: KotlinCompilationArguments val dependencyClasspath: List + val disambiguationClassifier: String? companion object { const val MAIN_COMPILATION_NAME = "main" diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt index 0728e0c4d86..b72137c5043 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt @@ -194,7 +194,10 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { } val jar = buildTargetJar(gradleTarget, project) val target = KotlinTargetImpl(gradleTarget.name, targetPresetName, disambiguationClassifier, platform, compilations, jar) - compilations.forEach { it.target = target } + compilations.forEach { + it.disambiguationClassifier = target.disambiguationClassifier + it.platform = target.platform + } return target } diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 index 4c8fae9bc8f..db79341fe22 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 @@ -185,7 +185,10 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { } val jar = buildTargetJar(gradleTarget, project) val target = KotlinTargetImpl(gradleTarget.name, null, disambiguationClassifier, platform, compilations, jar) - compilations.forEach { it.target = target } + compilations.forEach { + it.disambiguationClassifier = target.disambiguationClassifier + it.platform = target.platform + } return target } diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt index 7c21abb4f1f..f852b6d8320 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelImpl.kt @@ -7,7 +7,7 @@ package org.jetbrains.kotlin.gradle import java.io.File -class KotlinSourceSetImpl( +data class KotlinSourceSetImpl( override val name: String, override val languageSettings: KotlinLanguageSettings, override val sourceDirs: Set, @@ -15,6 +15,16 @@ class KotlinSourceSetImpl( override val dependencies: Set, override val dependsOnSourceSets: Set ) : KotlinSourceSet { + + constructor(kotlinSourceSet: KotlinSourceSet) : this( + kotlinSourceSet.name, + KotlinLanguageSettingsImpl(kotlinSourceSet.languageSettings), + HashSet(kotlinSourceSet.sourceDirs), + HashSet(kotlinSourceSet.resourceDirs), + kotlinSourceSet.dependencies.map { it.deepCopy() }.toSet(), + HashSet(kotlinSourceSet.dependsOnSourceSets) + ) + override var platform: KotlinPlatform = KotlinPlatform.COMMON internal set @@ -24,7 +34,7 @@ class KotlinSourceSetImpl( override fun toString() = name } -class KotlinLanguageSettingsImpl( +data class KotlinLanguageSettingsImpl( override val languageVersion: String?, override val apiVersion: String?, override val isProgressiveMode: Boolean, @@ -32,20 +42,38 @@ class KotlinLanguageSettingsImpl( override val experimentalAnnotationsInUse: Set, override val compilerPluginArguments: List, override val compilerPluginClasspath: Set -) : KotlinLanguageSettings +) : KotlinLanguageSettings { + constructor(settings: KotlinLanguageSettings) : this( + settings.languageVersion, + settings.apiVersion, + settings.isProgressiveMode, + settings.enabledLanguageFeatures, + settings.experimentalAnnotationsInUse, + settings.compilerPluginArguments, + settings.compilerPluginClasspath + ) +} -class KotlinCompilationOutputImpl( +data class KotlinCompilationOutputImpl( override val classesDirs: Set, override val effectiveClassesDir: File?, override val resourcesDir: File? -) : KotlinCompilationOutput +) : KotlinCompilationOutput { + constructor(output: KotlinCompilationOutput) : this( + HashSet(output.classesDirs), + output.effectiveClassesDir, + output.resourcesDir + ) +} -class KotlinCompilationArgumentsImpl( +data class KotlinCompilationArgumentsImpl( override val defaultArguments: List, override val currentArguments: List -) : KotlinCompilationArguments +) : KotlinCompilationArguments { + constructor(arguments: KotlinCompilationArguments) : this(ArrayList(arguments.defaultArguments), ArrayList(arguments.currentArguments)) +} -class KotlinCompilationImpl( +data class KotlinCompilationImpl( override val name: String, override val sourceSets: Collection, override val dependencies: Set, @@ -53,11 +81,25 @@ class KotlinCompilationImpl( override val arguments: KotlinCompilationArguments, override val dependencyClasspath: List ) : KotlinCompilation { - override lateinit var target: KotlinTarget + + // create deep copy + constructor(kotlinCompilation: KotlinCompilation) : this( + kotlinCompilation.name, + kotlinCompilation.sourceSets.map { KotlinSourceSetImpl(it) }.toList(), + kotlinCompilation.dependencies.map { it.deepCopy() }.toSet(), + KotlinCompilationOutputImpl(kotlinCompilation.output), + KotlinCompilationArgumentsImpl(kotlinCompilation.arguments), + ArrayList(kotlinCompilation.dependencyClasspath) + ) { + disambiguationClassifier = kotlinCompilation.disambiguationClassifier + platform = kotlinCompilation.platform + } + + override var disambiguationClassifier: String? = null + internal set + override lateinit var platform: KotlinPlatform internal set - override val platform: KotlinPlatform - get() = target.platform override val isTestModule: Boolean get() = name == KotlinCompilation.TEST_COMPILATION_NAME @@ -66,11 +108,11 @@ class KotlinCompilationImpl( override fun toString() = name } -class KotlinTargetJarImpl( +data class KotlinTargetJarImpl( override val archiveFile: File? ) : KotlinTargetJar -class KotlinTargetImpl( +data class KotlinTargetImpl( override val name: String, override val presetName: String?, override val disambiguationClassifier: String?, @@ -81,11 +123,11 @@ class KotlinTargetImpl( override fun toString() = name } -class ExtraFeaturesImpl( +data class ExtraFeaturesImpl( override val coroutinesState: String? ) : ExtraFeatures -class KotlinMPPGradleModelImpl( +data class KotlinMPPGradleModelImpl( override val sourceSets: Map, override val targets: Collection, override val extraFeatures: ExtraFeatures,