diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt index ebaa105187cba..90f69d989647c 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt @@ -166,6 +166,11 @@ abstract class AppendableSetBasicMap<KEY, E>( fun append(key: KEY, elements: Set<E>) { storage.append(key, elements) } + + @Synchronized + override fun clean() { + storage.clean() + } } abstract class BasicStringMap<VALUE>( diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt index 6b6d270cf7bb9..c2cfb04fc3539 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt @@ -41,40 +41,42 @@ open class BasicMapsOwner(val cachesDir: File) : Closeable { forEachMapSafe("flush", BasicMap<*, *>::flush) } - override fun close() { - forEachMapSafe("close", BasicMap<*, *>::close) - } - open fun deleteStorageFiles() { forEachMapSafe("deleteStorageFiles", BasicMap<*, *>::deleteStorageFiles) } /** - * DEPRECATED: This API should be removed because - * - It's not clear what [memoryCachesOnly] means. - * - In the past, when `memoryCachesOnly=true` we applied a small optimization: Checking - * [com.intellij.util.io.PersistentHashMap.isDirty] before calling [com.intellij.util.io.PersistentHashMap.force]. However, if that - * optimization is useful, it's better to always do it (perhaps inside the [com.intellij.util.io.PersistentHashMap.force] method - * itself) rather than doing it based on the value of this parameter. - * - * Instead, just call [flush] (without a parameter) directly. + * Please do not remove or modify this function. + * It is implementing [org.jetbrains.jps.incremental.storage.StorageOwner] interface and needed for correct JPS compilation. + */ + override fun close() { + forEachMapSafe("close", BasicMap<*, *>::close) + } + + /** + * Please do not remove or modify this function. + * It is implementing [org.jetbrains.jps.incremental.storage.StorageOwner] interface and needed for correct JPS compilation. */ fun flush(@Suppress("UNUSED_PARAMETER") memoryCachesOnly: Boolean) { flush() } /** - * DEPRECATED: This API should be removed because: - * - It's not obvious what "clean" means: It does not exactly describe the current implementation, and it also sounds similar to - * "clear" which means removing all the map entries, but this method does not do that. - * - This method currently calls [close] (and [deleteStorageFiles]). However, [close] is often already called separately and - * automatically, so this API makes it more likely for [close] to be accidentally called twice. + * Please do not remove or modify this function. + * It is implementing [org.jetbrains.jps.incremental.storage.StorageOwner] interface and needed for correct JPS compilation. + * Calling [org.jetbrains.kotlin.incremental.storage.BasicMapsOwner.close] here is unnecessary and will produce race conditions + * for JPS build because JPS can modify caches of dependant modules. * - * Instead, just call [close] and/or [deleteStorageFiles] explicitly. + * More context: + * 1) While compiling module Foo, thread A can open caches of dependent module Bar + * 2) When we will compile module Bar in thread B we can decide to rebuild the module (e.g. configuration of facet changed) + * 3) Thread B will call `clean` action on caches of module Bar + * 4) If `clean` action also call `close` action, + * it will close opened map and will make it unusable when it tries to add info after recompilation, + * which will cause a "Storage already closed" exception. */ fun clean() { - close() - deleteStorageFiles() + forEachMapSafe("clean", BasicMap<*, *>::clean) } @Synchronized diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt index 0af8a0cbef337..4f3fcdf047f06 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt @@ -137,6 +137,10 @@ open class InMemoryStorage<KEY, VALUE>( storage.close() } + @Synchronized + override fun clean() { + storage.clean() + } } /** [InMemoryStorage] where a map entry's value is a [Collection] of elements of type [E]. */ diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt index e9be49c692a5a..20d224c36eb24 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.incremental.storage import com.intellij.util.CommonProcessors import com.intellij.util.io.AppendablePersistentMap import com.intellij.util.io.DataExternalizer +import com.intellij.util.io.IOUtil import com.intellij.util.io.KeyDescriptor import com.intellij.util.io.PersistentHashMap import org.jetbrains.kotlin.incremental.IncrementalCompilationContext @@ -27,6 +28,7 @@ import java.io.DataInput import java.io.DataInputStream import java.io.DataOutput import java.io.File +import java.io.IOException /** * [PersistentStorage] which delegates operations to a [PersistentHashMap]. Note that the [PersistentHashMap] is created lazily (only when @@ -90,9 +92,24 @@ open class LazyStorage<KEY, VALUE>( @Synchronized override fun close() { - storage?.close() + try { + storage?.close() + } finally { + storage = null + } } + @Synchronized + override fun clean() { + try { + storage?.close() + } finally { + storage = null + if (!IOUtil.deleteAllFilesStartingWith(storageFile)) { + throw IOException("Could not delete internal storage: ${storageFile.absolutePath}") + } + } + } } /** [LazyStorage] where a map entry's value is a [Collection] of elements of type [E]. */ diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt index ab397a6d85d79..5833068d7ae8b 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt @@ -48,6 +48,8 @@ interface PersistentStorage<KEY, VALUE> : Closeable { /** Writes any remaining in-memory changes to [storageFile] ([flush]) and closes this map. */ override fun close() + + fun clean() } /** [PersistentStorage] where a map entry's value is a [Collection] of elements of type [E]. */ @@ -108,6 +110,11 @@ abstract class PersistentStorageWrapper<KEY, VALUE>( override fun close() { storage.close() } + + @Synchronized + override fun clean() { + storage.clean() + } } /** [PersistentStorageWrapper] where a map entry's value is a [Collection] of elements of type [E]. */ diff --git a/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt b/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt index a82f5e0de9971..4b362f6d495a6 100644 --- a/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt +++ b/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt @@ -50,6 +50,8 @@ private class InMemoryStorageWrapperMock : InMemoryStorageInterface<Any, Any> { override fun get(key: Any) = null override fun contains(key: Any) = false + + override fun clean() {} } abstract class BaseCompilationTransactionTest {