[Gradle, native] Don't set system properties when the compiler runs in daemon

Changing system properties during parallel in-process compiler
execution may cause data races (e.g. see KT-37362, KT-37442).

This commit abandons using system properties when the compiler
is executed in process.

Issue #KT-37550 fixed
This commit is contained in:
Ilya Matveev
2020-05-13 18:40:05 +07:00
committed by Ilya Matveev
parent f88722ae76
commit 8d77ec83c6
2 changed files with 24 additions and 46 deletions
@@ -21,11 +21,11 @@ internal abstract class KotlinToolRunner(
abstract val mainClass: String
open val daemonEntryPoint: String get() = "main"
open val environment: Map<String, String> = emptyMap()
open val environmentBlacklist: Set<String> = emptySet()
open val execEnvironment: Map<String, String> = emptyMap()
open val execEnvironmentBlacklist: Set<String> = emptySet()
open val systemProperties: Map<String, String> = emptyMap()
open val systemPropertiesBlacklist: Set<String> = setOf("java.endorsed.dirs")
open val execSystemProperties: Map<String, String> = emptyMap()
open val execSystemPropertiesBlacklist: Set<String> = setOf("java.endorsed.dirs")
abstract val classpath: Set<File>
open fun checkClasspath(): Unit = check(classpath.isNotEmpty()) { "Classpath of the tool is empty: $displayName" }
@@ -79,21 +79,19 @@ internal abstract class KotlinToolRunner(
spec.systemProperties(
System.getProperties().asSequence()
.map { (k, v) -> k.toString() to v.toString() }
.filter { (k, _) -> k !in systemPropertiesBlacklist }
.filter { (k, _) -> k !in execSystemPropertiesBlacklist }
.escapeQuotesForWindows()
.toMap()
)
spec.systemProperties(systemProperties)
environmentBlacklist.forEach { spec.environment.remove(it) }
spec.environment(environment)
spec.systemProperties(execSystemProperties)
execEnvironmentBlacklist.forEach { spec.environment.remove(it) }
spec.environment(execEnvironment)
spec.args(transformArgs(args))
}
}
// TODO: Make it private again once KT-37550 is fixed.
protected open fun runInProcess(args: List<String>) {
val oldProperties = setUpSystemProperties()
try {
val mainClass = getIsolatedClassLoader().loadClass(mainClass)
val entryPoint = mainClass.methods.single { it.name == daemonEntryPoint }
@@ -101,32 +99,6 @@ internal abstract class KotlinToolRunner(
entryPoint.invoke(null, transformArgs(args).toTypedArray())
} catch (t: InvocationTargetException) {
throw t.targetException
} finally {
restoreSystemProperties(oldProperties)
}
}
private fun setUpSystemProperties(): Map<String, String?> {
val oldProperties = mutableMapOf<String, String?>()
systemProperties.forEach { (k, v) -> oldProperties[k] = System.getProperty(v) }
System.getProperties().toList()
.map { (k, v) -> k.toString() to v.toString() }
.filter { (k, _) -> k in systemPropertiesBlacklist }
.forEach { (k, v) ->
oldProperties[k] = v
System.clearProperty(k)
}
systemProperties.forEach { (k, v) -> System.setProperty(k, v) }
return oldProperties
}
private fun restoreSystemProperties(oldProperties: Map<String, String?>) {
oldProperties.forEach { (k, v) ->
if (v == null) System.clearProperty(k) else System.setProperty(k, v)
}
}
@@ -9,6 +9,7 @@ import org.gradle.api.Project
import org.jetbrains.kotlin.cli.common.messages.MessageRenderer
import org.jetbrains.kotlin.gradle.dsl.NativeCacheKind
import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider
import org.jetbrains.kotlin.gradle.plugin.mpp.isAtLeast
import org.jetbrains.kotlin.gradle.utils.NativeCompilerDownloader
import org.jetbrains.kotlin.konan.CompilerVersion
import org.jetbrains.kotlin.konan.target.HostManager
@@ -43,8 +44,7 @@ internal abstract class KotlinNativeToolRunner(
final override val daemonEntryPoint get() = "daemonMain"
// We need to unset some environment variables which are set by XCode and may potentially affect the tool executed.
override val environment = mapOf("LIBCLANG_DISABLE_CRASH_RECOVERY" to "1")
final override val environmentBlacklist: Set<String> by lazy {
final override val execEnvironmentBlacklist: Set<String> by lazy {
HashSet<String>().also { collector ->
KotlinNativeToolRunner::class.java.getResourceAsStream("/env_blacklist")?.let { stream ->
stream.reader().use { r -> r.forEachLine { collector.add(it) } }
@@ -52,11 +52,17 @@ internal abstract class KotlinNativeToolRunner(
}
}
final override val systemProperties by lazy {
mapOf(
"konan.home" to project.konanHome,
MessageRenderer.PROPERTY_KEY to MessageRenderer.GRADLE_STYLE.name
)
final override val execSystemProperties by lazy {
// Still set konan.home for versions prior to 1.4-M3.
val konanHomeRequired = project.konanVersion.let {
!it.isAtLeast(1, 4, 0) ||
it.toString(showMeta = false, showBuild = false) in listOf("1.4-M1", "1.4-M2")
}
listOfNotNull(
if (konanHomeRequired) "konan.home" to project.konanHome else null,
MessageRenderer.PROPERTY_KEY to MessageRenderer.GRADLE_STYLE.name
).toMap()
}
final override val classpath by lazy {
@@ -127,12 +133,12 @@ internal abstract class KotlinNativeToolRunner(
internal abstract class AbstractKotlinNativeCInteropRunner(toolName: String, project: Project) : KotlinNativeToolRunner(toolName, project) {
override val mustRunViaExec get() = true
override val environment by lazy {
override val execEnvironment by lazy {
val llvmExecutablesPath = llvmExecutablesPath
if (llvmExecutablesPath != null)
super.environment + ("PATH" to "$llvmExecutablesPath;${System.getenv("PATH")}")
super.execEnvironment + ("PATH" to "$llvmExecutablesPath;${System.getenv("PATH")}")
else
super.environment
super.execEnvironment
}
private val llvmExecutablesPath: String? by lazy {