From 8ab9366ae5a5576bb8cee959b80610b98d6639a2 Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Thu, 22 Feb 2018 01:45:49 +0300 Subject: [PATCH] Synchronize access to lookup storage in JPS Storage operations are synchronized already, but BuildDataManager.getStorage is not synchronized, so maybe it is possible to create multiple LookupStorage instances. #KT-22535 #KT-22995 --- .../jps/build/AbstractIncrementalJpsTest.kt | 10 +++++----- .../kotlin/jps/build/KotlinBuilder.kt | 20 +++++++++---------- .../jps/build/KotlinConstantSearchProvider.kt | 8 ++++---- .../jps/incremental/JpsLookupStorage.kt | 11 ++++++++-- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt index 47475dbf6c3..553c44b7899 100644 --- a/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt +++ b/jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt @@ -47,9 +47,8 @@ import org.jetbrains.kotlin.config.IncrementalCompilation import org.jetbrains.kotlin.incremental.CacheVersion import org.jetbrains.kotlin.incremental.LookupSymbol import org.jetbrains.kotlin.incremental.testingUtils.* -import org.jetbrains.kotlin.jps.incremental.JpsLookupStorageProvider -import org.jetbrains.kotlin.jps.incremental.KotlinDataContainerTarget import org.jetbrains.kotlin.jps.incremental.getKotlinCache +import org.jetbrains.kotlin.jps.incremental.withLookupStorage import org.jetbrains.kotlin.test.KotlinTestUtils import org.jetbrains.kotlin.utils.Printer import org.jetbrains.kotlin.utils.keysToMap @@ -302,9 +301,10 @@ abstract class AbstractIncrementalJpsTest( p.println("Begin of Lookup Maps") p.println() - val lookupStorage = project.dataManager.getStorage(KotlinDataContainerTarget, JpsLookupStorageProvider) - lookupStorage.forceGC() - p.print(lookupStorage.dump(lookupsDuringTest)) + project.dataManager.withLookupStorage { lookupStorage -> + lookupStorage.forceGC() + p.print(lookupStorage.dump(lookupsDuringTest)) + } p.println() p.println("End of Lookup Maps") diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilder.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilder.kt index a851f3b1609..21650505dd2 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilder.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilder.kt @@ -380,7 +380,7 @@ class KotlinBuilder : ModuleLevelBuilder(BuilderCategory.SOURCE_PROCESSOR) { rebuildAfterCacheVersionChanged[target] = true } - dataManager.getStorage(KotlinDataContainerTarget, JpsLookupStorageProvider).clean() + dataManager.withLookupStorage { it.clean() } return } CacheVersion.Action.REBUILD_CHUNK -> { @@ -405,7 +405,7 @@ class KotlinBuilder : ModuleLevelBuilder(BuilderCategory.SOURCE_PROCESSOR) { } CacheVersion.Action.CLEAN_DATA_CONTAINER -> { LOG.info("Clearing lookup cache") - dataManager.getStorage(KotlinDataContainerTarget, JpsLookupStorageProvider).clean() + dataManager.withLookupStorage { it.clean() } cacheVersionsProvider.dataContainerVersion().clean() } else -> { @@ -617,12 +617,11 @@ class KotlinBuilder : ModuleLevelBuilder(BuilderCategory.SOURCE_PROCESSOR) { ) { if (lookupTracker !is LookupTrackerImpl) throw AssertionError("Lookup tracker is expected to be LookupTrackerImpl, got ${lookupTracker::class.java}") - val lookupStorage = dataManager.getStorage(KotlinDataContainerTarget, JpsLookupStorageProvider) - val removedFiles = chunk.targets.flatMap { KotlinSourceFileCollector.getRemovedKotlinFiles(dirtyFilesHolder, it) } - lookupStorage.removeLookupsFrom(filesToCompile.values().asSequence() + removedFiles.asSequence()) - - lookupStorage.addAll(lookupTracker.lookups.entrySet(), lookupTracker.pathInterner.values) + dataManager.withLookupStorage { lookupStorage -> + lookupStorage.removeLookupsFrom(filesToCompile.values().asSequence() + removedFiles.asSequence()) + lookupStorage.addAll(lookupTracker.lookups.entrySet(), lookupTracker.pathInterner.values) + } } // if null is returned, nothing was done @@ -818,15 +817,16 @@ private fun ChangesCollector.processChangesUsingLookups( fsOperations: FSOperationsHelper, caches: Iterable ) { - val lookupStorage = dataManager.getStorage(KotlinDataContainerTarget, JpsLookupStorageProvider) val allCaches = caches.flatMap { it.thisWithDependentCaches } val reporter = JpsICReporter() reporter.report { "Start processing changes" } val (dirtyLookupSymbols, dirtyClassFqNames) = getDirtyData(allCaches, reporter) - val dirtyFiles = mapLookupSymbolsToFiles(lookupStorage, dirtyLookupSymbols, reporter) + - mapClassesFqNamesToFiles(allCaches, dirtyClassFqNames, reporter) + val dirtyFilesFromLookups = dataManager.withLookupStorage { + mapLookupSymbolsToFiles(it, dirtyLookupSymbols, reporter) + } + val dirtyFiles = dirtyFilesFromLookups + mapClassesFqNamesToFiles(allCaches, dirtyClassFqNames, reporter) fsOperations.markInChunkOrDependents(dirtyFiles.asIterable(), excludeFiles = compiledFiles) reporter.report { "End of processing changes" } diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinConstantSearchProvider.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinConstantSearchProvider.kt index beff6961bf5..ddd34a6252e 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinConstantSearchProvider.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinConstantSearchProvider.kt @@ -10,8 +10,7 @@ import org.jetbrains.jps.builders.java.ConstantSearchProvider import org.jetbrains.jps.builders.java.dependencyView.Callbacks import org.jetbrains.jps.incremental.CompileContext import org.jetbrains.kotlin.incremental.LookupSymbol -import org.jetbrains.kotlin.jps.incremental.JpsLookupStorageProvider -import org.jetbrains.kotlin.jps.incremental.KotlinDataContainerTarget +import org.jetbrains.kotlin.jps.incremental.withLookupStorage import java.io.File import java.util.concurrent.Future @@ -30,8 +29,9 @@ class KotlinLookupConstantSearch(context: CompileContext) : Callbacks.ConstantAf fieldRemoved: Boolean, accessChanged: Boolean ): Future { - val storage = dataManager.getStorage(KotlinDataContainerTarget, JpsLookupStorageProvider) - val paths = storage.get(LookupSymbol(name = fieldName, scope = ownerClassName)) + val paths = dataManager.withLookupStorage { storage -> + storage.get(LookupSymbol(name = fieldName, scope = ownerClassName)) + } return FixedFuture(Callbacks.ConstantAffection(paths.map(::File))) } } \ No newline at end of file diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/JpsLookupStorage.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/JpsLookupStorage.kt index 3268002254b..f67fab4be2f 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/JpsLookupStorage.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/JpsLookupStorage.kt @@ -17,12 +17,19 @@ package org.jetbrains.kotlin.jps.incremental import org.jetbrains.jps.builders.storage.StorageProvider +import org.jetbrains.jps.incremental.storage.BuildDataManager import org.jetbrains.jps.incremental.storage.StorageOwner import org.jetbrains.kotlin.incremental.LookupStorage import java.io.File -object JpsLookupStorageProvider : StorageProvider() { +@Synchronized +fun BuildDataManager.withLookupStorage(fn: (LookupStorage) -> T): T { + val lookupStorage = getStorage(KotlinDataContainerTarget, JpsLookupStorageProvider) + return fn(lookupStorage) +} + +private object JpsLookupStorageProvider : StorageProvider() { override fun createStorage(targetDataDir: File): JpsLookupStorage = JpsLookupStorage(targetDataDir) } -class JpsLookupStorage(targetDataDir: File) : StorageOwner, LookupStorage(targetDataDir) +private class JpsLookupStorage(targetDataDir: File) : StorageOwner, LookupStorage(targetDataDir)