Fix synchronization when working with IC caches
IC caches could be modified and read from different threads. In JPS builder these threads are RMI worker (invoked from Compiler Daemon) and JPS worker thread. Proper synchronization fixes cases when caches could become broken. #KT-42265 Fixed #KT-42194 Fixed #KT-42265 Fixed #KT-42937 Fixed
This commit is contained in:
@@ -176,8 +176,8 @@ abstract class AbstractIncrementalCache<ClassName>(
|
||||
protected class ClassFqNameToSourceMap(
|
||||
storageFile: File,
|
||||
private val pathConverter: FileToPathConverter
|
||||
) :
|
||||
BasicStringMap<String>(storageFile, EnumeratorStringDescriptor(), PathStringDescriptor) {
|
||||
) : BasicStringMap<String>(storageFile, EnumeratorStringDescriptor(), PathStringDescriptor) {
|
||||
|
||||
operator fun set(fqName: FqName, sourceFile: File) {
|
||||
storage[fqName.asString()] = pathConverter.toPath(sourceFile)
|
||||
}
|
||||
|
||||
@@ -272,6 +272,7 @@ open class IncrementalJvmCache(
|
||||
|
||||
private inner class ProtoMap(storageFile: File) : BasicStringMap<ProtoMapValue>(storageFile, ProtoMapValueExternalizer) {
|
||||
|
||||
@Synchronized
|
||||
fun process(kotlinClass: LocalFileKotlinClass, changesCollector: ChangesCollector) {
|
||||
return put(kotlinClass, changesCollector)
|
||||
}
|
||||
@@ -283,10 +284,12 @@ open class IncrementalJvmCache(
|
||||
// from files compiled during last round.
|
||||
// However there is no need to compare old and new data in this case
|
||||
// (also that would fail with exception).
|
||||
@Synchronized
|
||||
fun storeModuleMapping(className: JvmClassName, bytes: ByteArray) {
|
||||
storage[className.internalName] = ProtoMapValue(isPackageFacade = false, bytes = bytes, strings = emptyArray())
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
private fun put(kotlinClass: LocalFileKotlinClass, changesCollector: ChangesCollector) {
|
||||
val header = kotlinClass.classHeader
|
||||
|
||||
@@ -309,6 +312,7 @@ open class IncrementalJvmCache(
|
||||
operator fun get(className: JvmClassName): ProtoMapValue? =
|
||||
storage[className.internalName]
|
||||
|
||||
@Synchronized
|
||||
fun remove(className: JvmClassName, changesCollector: ChangesCollector) {
|
||||
val key = className.internalName
|
||||
val oldValue = storage[key] ?: return
|
||||
@@ -325,6 +329,8 @@ open class IncrementalJvmCache(
|
||||
|
||||
private inner class JavaSourcesProtoMap(storageFile: File) :
|
||||
BasicStringMap<SerializedJavaClass>(storageFile, JavaClassProtoMapValueExternalizer) {
|
||||
|
||||
@Synchronized
|
||||
fun process(jvmClassName: JvmClassName, newData: SerializedJavaClass, changesCollector: ChangesCollector) {
|
||||
val key = jvmClassName.internalName
|
||||
val oldData = storage[key]
|
||||
@@ -336,6 +342,7 @@ open class IncrementalJvmCache(
|
||||
)
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
fun remove(className: JvmClassName, changesCollector: ChangesCollector) {
|
||||
val key = className.internalName
|
||||
val oldValue = storage[key] ?: return
|
||||
@@ -375,6 +382,7 @@ open class IncrementalJvmCache(
|
||||
operator fun contains(className: JvmClassName): Boolean =
|
||||
className.internalName in storage
|
||||
|
||||
@Synchronized
|
||||
fun process(kotlinClass: LocalFileKotlinClass, changesCollector: ChangesCollector) {
|
||||
val key = kotlinClass.className.internalName
|
||||
val oldMap = storage[key] ?: emptyMap()
|
||||
@@ -391,6 +399,7 @@ open class IncrementalJvmCache(
|
||||
}
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
fun remove(className: JvmClassName) {
|
||||
storage.remove(className.internalName)
|
||||
}
|
||||
@@ -523,6 +532,7 @@ open class IncrementalJvmCache(
|
||||
return result
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
fun process(kotlinClass: LocalFileKotlinClass, changesCollector: ChangesCollector) {
|
||||
val key = kotlinClass.className.internalName
|
||||
val oldMap = storage[key] ?: emptyMap()
|
||||
@@ -548,6 +558,7 @@ open class IncrementalJvmCache(
|
||||
private fun functionNameBySignature(signature: String): String =
|
||||
signature.substringBefore("(")
|
||||
|
||||
@Synchronized
|
||||
fun remove(className: JvmClassName) {
|
||||
storage.remove(className.internalName)
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ open class BasicMapsOwner(val cachesDir: File) {
|
||||
protected val String.storageFile: File
|
||||
get() = File(cachesDir, this + "." + CACHE_EXTENSION)
|
||||
|
||||
@Synchronized
|
||||
protected fun <K, V, M : BasicMap<K, V>> registerMap(map: M): M {
|
||||
maps.add(map)
|
||||
return map
|
||||
@@ -47,6 +48,7 @@ open class BasicMapsOwner(val cachesDir: File) {
|
||||
forEachMapSafe("flush") { it.flush(memoryCachesOnly) }
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
private fun forEachMapSafe(actionName: String, action: (BasicMap<*, *>) -> Unit) {
|
||||
val actionExceptions = LinkedHashMap<String, Exception>()
|
||||
maps.forEach {
|
||||
@@ -66,5 +68,6 @@ open class BasicMapsOwner(val cachesDir: File) {
|
||||
}
|
||||
|
||||
@TestOnly
|
||||
@Synchronized
|
||||
fun dump(): String = maps.joinToString("\n\n") { it.dump() }
|
||||
}
|
||||
@@ -17,9 +17,11 @@
|
||||
package org.jetbrains.kotlin.incremental.storage
|
||||
|
||||
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 java.io.File
|
||||
import java.io.IOException
|
||||
|
||||
|
||||
/**
|
||||
@@ -30,7 +32,6 @@ class CachingLazyStorage<K, V>(
|
||||
private val keyDescriptor: KeyDescriptor<K>,
|
||||
private val valueExternalizer: DataExternalizer<V>
|
||||
) : LazyStorage<K, V> {
|
||||
@Volatile
|
||||
private var storage: PersistentHashMap<K, V>? = null
|
||||
|
||||
@Synchronized
|
||||
@@ -80,8 +81,10 @@ class CachingLazyStorage<K, V>(
|
||||
try {
|
||||
storage?.close()
|
||||
} finally {
|
||||
PersistentHashMap.deleteFilesStartingWith(storageFile)
|
||||
storage = null
|
||||
if (!IOUtil.deleteAllFilesStartingWith(storageFile)) {
|
||||
throw IOException("Could not delete internal storage: ${storageFile.absolutePath}")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -20,18 +20,18 @@ import org.jetbrains.kotlin.incremental.dumpCollection
|
||||
import org.jetbrains.kotlin.name.FqName
|
||||
import java.io.File
|
||||
|
||||
internal open class ClassOneToManyMap(
|
||||
storageFile: File
|
||||
) : BasicStringMap<Collection<String>>(storageFile, StringCollectionExternalizer) {
|
||||
internal open class ClassOneToManyMap(storageFile: File) : BasicStringMap<Collection<String>>(storageFile, StringCollectionExternalizer) {
|
||||
override fun dumpValue(value: Collection<String>): String = value.dumpCollection()
|
||||
|
||||
@Synchronized
|
||||
fun add(key: FqName, value: FqName) {
|
||||
storage.append(key.asString(), listOf(value.asString()))
|
||||
}
|
||||
|
||||
operator fun get(key: FqName): Collection<FqName> =
|
||||
storage[key.asString()]?.map(::FqName) ?: setOf()
|
||||
storage[key.asString()]?.map(::FqName) ?: setOf()
|
||||
|
||||
@Synchronized
|
||||
operator fun set(key: FqName, values: Collection<FqName>) {
|
||||
if (values.isEmpty()) {
|
||||
remove(key)
|
||||
@@ -41,10 +41,14 @@ internal open class ClassOneToManyMap(
|
||||
storage[key.asString()] = values.map(FqName::asString)
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
fun remove(key: FqName) {
|
||||
storage.remove(key.asString())
|
||||
}
|
||||
|
||||
// Access to caches could be done from multiple threads (e.g. JPS worker and RMI). The underlying collection is already synchronized,
|
||||
// thus we need synchronization of this method and all modification methods.
|
||||
@Synchronized
|
||||
fun removeValues(key: FqName, removed: Set<FqName>) {
|
||||
val notRemoved = this[key].filter { it !in removed }
|
||||
this[key] = notRemoved
|
||||
|
||||
@@ -17,9 +17,11 @@
|
||||
package org.jetbrains.kotlin.incremental.storage
|
||||
|
||||
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 java.io.File
|
||||
import java.io.IOException
|
||||
|
||||
|
||||
class NonCachingLazyStorage<K, V>(
|
||||
@@ -27,7 +29,6 @@ class NonCachingLazyStorage<K, V>(
|
||||
private val keyDescriptor: KeyDescriptor<K>,
|
||||
private val valueExternalizer: DataExternalizer<V>
|
||||
) : LazyStorage<K, V> {
|
||||
@Volatile
|
||||
private var storage: PersistentHashMap<K, V>? = null
|
||||
|
||||
@Synchronized
|
||||
@@ -76,11 +77,12 @@ class NonCachingLazyStorage<K, V>(
|
||||
override fun clean() {
|
||||
try {
|
||||
storage?.close()
|
||||
} catch (ignored: Throwable) {
|
||||
} finally {
|
||||
storage = null
|
||||
if (!IOUtil.deleteAllFilesStartingWith(storageFile)) {
|
||||
throw IOException("Could not delete internal storage: ${storageFile.absolutePath}")
|
||||
}
|
||||
}
|
||||
|
||||
PersistentHashMap.deleteFilesStartingWith(storageFile)
|
||||
storage = null
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
|
||||
Reference in New Issue
Block a user