azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Core. Fix data-race in ApplicationEnvironment initialization

Browse Source 
getOrCreateApplicationEnvironment is main entrypoint to create
ApplicationEnvironment in production and compiler tests
It is subject to be called concurrently, that's why
APPLICATION_LOCK exists

ApplicationEnvironment itself hosts Application from idea-core
It is actually singleton, that is subject to be disposed, once all
operations referencing it completes

To properly dispose ApplicationEnvironment when there is no references
left, we maintain reference counter aka ourProjectCount

Originally, there was data-race caused by the fact, that ourProjectCount
was updated after publication of application

Linear, data-race occurs in following order
T1: getOrCreateApplicationEnvironment returns application
T2: Disposes its reference to application, causing ourProjectCount to
reach zero, and disposing application that is already available to T1
T1: Updates counter, but its application already disposed
This commit is contained in:
Simon Ogorodnik
2022-08-12 23:06:54 +02:00
committed by teamcity
parent a6c7d4c0c6
commit f2dee2bf85
2 changed files with 27 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/Properties.kt
+11
View File
@@ -33,6 +33,17 @@ enum class CompilerSystemProperties(val property: String, val alwaysDirectAccess
DAEMON_RMI_SOCKET_BACKLOG_SIZE_PROPERTY("kotlin.daemon.socket.backlog.size"),
DAEMON_RMI_SOCKET_CONNECT_ATTEMPTS_PROPERTY("kotlin.daemon.socket.connect.attempts"),
DAEMON_RMI_SOCKET_CONNECT_INTERVAL_PROPERTY("kotlin.daemon.socket.connect.interval"),
/**
* Only specify that property if you want to avoid multiple initializations/disposals of KotlinCoreApplicationEnvironment
*
* **If not specified**, KotlinCoreApplicationEnvironment **WILL dispose** after all concurrent compilations finish,
* and will be re-initialized when starting new compilation
*
* If specified, KotlinCoreApplicationEnvironment will **NOT dispose** after all concurrent compilations finished
* and will be pinned with hard-reference
* **THIS LEAKS MEMORY**
* */
KOTLIN_COMPILER_ENVIRONMENT_KEEPALIVE_PROPERTY("kotlin.environment.keepalive"),
COMPILE_DAEMON_CUSTOM_RUN_FILES_PATH_FOR_TESTS("kotlin.daemon.custom.run.files.path.for.tests"),
COMPILE_INCREMENTAL_WITH_ARTIFACT_TRANSFORM("kotlin.incremental.useClasspathSnapshot"),
compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCoreEnvironment.kt
+16 -31
View File
@@ -443,9 +443,6 @@ class KotlinCoreEnvironment private constructor(
val projectEnv = ProjectEnvironment(parentDisposable, appEnv, configuration)
val environment = KotlinCoreEnvironment(projectEnv, configuration, configFiles)
synchronized(APPLICATION_LOCK) {
ourProjectCount++
}
return environment
}
@@ -453,15 +450,7 @@ class KotlinCoreEnvironment private constructor(
fun createForProduction(
projectEnvironment: ProjectEnvironment, configuration: CompilerConfiguration, configFiles: EnvironmentConfigFiles
): KotlinCoreEnvironment {
val environment = KotlinCoreEnvironment(projectEnvironment, configuration, configFiles)
if (projectEnvironment.environment == applicationEnvironment) {
// accounting for core environment disposing
synchronized(APPLICATION_LOCK) {
ourProjectCount++
}
}
return environment
return KotlinCoreEnvironment(projectEnvironment, configuration, configFiles)
}
@TestOnly
@@ -506,14 +495,7 @@ class KotlinCoreEnvironment private constructor(
): KotlinCoreApplicationEnvironment {
synchronized(APPLICATION_LOCK) {
if (ourApplicationEnvironment == null) {
val disposable = if (unitTestMode) {
parentDisposable
} else {
// TODO this is a memory leak in the compiler, as this Disposable is not registered and thus is never disposed
// but using parentDisposable as disposable in compiler, causes access to application extension points after
// they was disposed, this needs to be fixed
Disposer.newDisposable("Disposable for the KotlinCoreApplicationEnvironment")
}
val disposable = Disposer.newDisposable("Disposable for the KotlinCoreApplicationEnvironment")
ourApplicationEnvironment = createApplicationEnvironment(disposable, configuration, unitTestMode)
ourProjectCount = 0
Disposer.register(disposable, Disposable {
@@ -522,18 +504,21 @@ class KotlinCoreEnvironment private constructor(
}
})
}
// Disposing of the environment is unsafe in production then parallel builds are enabled, but turning it off universally
// breaks a lot of tests, therefore it is disabled for production and enabled for tests
if (CompilerSystemProperties.KOTLIN_COMPILER_ENVIRONMENT_KEEPALIVE_PROPERTY.value.toBooleanLenient() != true) {
// JPS may run many instances of the compiler in parallel (there's an option for compiling independent modules in parallel in IntelliJ)
// All projects share the same ApplicationEnvironment, and when the last project is disposed, the ApplicationEnvironment is disposed as well
Disposer.register(parentDisposable, Disposable {
synchronized(APPLICATION_LOCK) {
if (--ourProjectCount <= 0) {
disposeApplicationEnvironment()
try {
// Do not use this property unless you sure need it, causes Application to MEMORY LEAK
// Only valid use-case is when Application should be cached to avoid
// initialization costs
if (CompilerSystemProperties.KOTLIN_COMPILER_ENVIRONMENT_KEEPALIVE_PROPERTY.value.toBooleanLenient() != true) {
Disposer.register(parentDisposable, Disposable {
synchronized(APPLICATION_LOCK) {
if (--ourProjectCount <= 0) {
disposeApplicationEnvironment()
}
}
}
})
})
}
} finally {
ourProjectCount++
}
return ourApplicationEnvironment!!