KT-57154: Fix JRT-FS contents served for wrong JDK home on JDK 9+

If the compiler runtime JDK is 9+,
it will already contain JrtFileSystemProvider and won't use provided
classloader
In order to fix KT-57154 we need to provide "java.home" argument to
newFileSystem

In order to reduce the severity of the leak in KT-56789 we cache
instances of FileSystem itself forever
Otherwise, each invocation of newFileSystem on JDK 9+ will leak
classloader, which is created deep inside the JDK code

Add unit test for JRT-FS contents served through CoreJrtFs

Add Gradle Integration test to 
test if the daemon correctly reads JDK contents from the specified 
toolchain and not from its runtime JDK

^KT-57154
Regression test for ^KT-57077
This commit is contained in:
Simon Ogorodnik
2023-03-28 10:38:46 +00:00
committed by Space Team
parent 563781a246
commit ae32eff543
7 changed files with 158 additions and 17 deletions
@@ -20,31 +20,18 @@ import com.intellij.openapi.vfs.DeprecatedVirtualFileSystem
import com.intellij.openapi.vfs.StandardFileSystems
import com.intellij.openapi.vfs.VirtualFile
import com.intellij.util.containers.ConcurrentFactoryMap
import com.intellij.util.containers.ContainerUtil
import com.intellij.util.io.URLUtil
import java.io.File
import java.net.URI
import java.net.URLClassLoader
import java.nio.file.FileSystem
import java.nio.file.FileSystems
// There's JrtFileSystem in idea-full which we can't use in the compiler because it depends on NewVirtualFileSystem, absent in intellij-core
class CoreJrtFileSystem : DeprecatedVirtualFileSystem() {
private val roots =
ConcurrentFactoryMap.createMap<String, CoreJrtVirtualFile?> { jdkHomePath ->
val jdkHome = File(jdkHomePath)
val rootUri = URI.create(StandardFileSystems.JRT_PROTOCOL + ":/")
val jrtFsJar = loadJrtFsJar(jdkHome) ?: return@createMap null
/*
This ClassLoader actually lives as long as current thread due to ThreadLocal leak in jrt-fs,
See https://bugs.openjdk.java.net/browse/JDK-8260621
So that cache allows us to avoid creating too many classloaders for same JDK and reduce severity of that leak
*/
val classLoader = globalJrtFsClassLoaderCache.computeIfAbsent(jrtFsJar) {
URLClassLoader(arrayOf(jrtFsJar.toURI().toURL()), null)
}
val fileSystem = FileSystems.newFileSystem(rootUri, emptyMap<String, Nothing>(), classLoader)
val fileSystem = globalJrtFsCache[jdkHomePath] ?: return@createMap null
CoreJrtVirtualFile(this, jdkHomePath, fileSystem.getPath(""), parent = null)
}
@@ -84,6 +71,26 @@ class CoreJrtFileSystem : DeprecatedVirtualFileSystem() {
return Pair(localPath, pathInJar)
}
private val globalJrtFsClassLoaderCache = ContainerUtil.createConcurrentWeakValueMap<File, URLClassLoader>()
private val globalJrtFsCache = ConcurrentFactoryMap.createMap<String, FileSystem?> { jdkHomePath ->
val jdkHome = File(jdkHomePath)
val jrtFsJar = loadJrtFsJar(jdkHome) ?: return@createMap null
val rootUri = URI.create(StandardFileSystems.JRT_PROTOCOL + ":/")
/*
The ClassLoader, that was used to load JRT FS Provider actually lives as long as current thread due to ThreadLocal leak in jrt-fs,
See https://bugs.openjdk.java.net/browse/JDK-8260621
So that cache allows us to avoid creating too many classloaders for same JDK and reduce severity of that leak
*/
if (isAtLeastJava9()) {
// If the runtime JDK is set to 9+ it has JrtFileSystemProvider,
// but to load proper jrt-fs (one that is pointed by jdkHome) we should provide "java.home" path
FileSystems.newFileSystem(rootUri, mapOf("java.home" to jdkHome.absolutePath))
} else {
val classLoader = URLClassLoader(arrayOf(jrtFsJar.toURI().toURL()), null)
// If the runtime JDK is set to <9, there are no JrtFileSystemProvider,
// we should create classloader with jrt-fs.jar, and DO NOT NEED to pass "java.home" path,
// as otherwise it will incur additional classloader creation
FileSystems.newFileSystem(rootUri, emptyMap<String, Nothing>(), classLoader)
}
}
}
}
@@ -0,0 +1,77 @@
/*
* Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
* Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
*/
package org.jetbrains.kotlin.test.jvm.compiler
import com.intellij.openapi.Disposable
import com.intellij.openapi.util.Disposer
import com.intellij.openapi.vfs.VirtualFile
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.jetbrains.kotlin.config.CompilerConfiguration
import org.jetbrains.kotlin.config.JVMConfigurationKeys
import org.jetbrains.kotlin.test.KtAssert.assertEquals
import org.jetbrains.kotlin.test.TestJdkKind
import org.jetbrains.kotlin.test.services.configuration.JvmEnvironmentConfigurator
import org.jetbrains.org.objectweb.asm.ClassReader
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.tree.ClassNode
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
class CoreJrtFsTest {
private fun checkClassVersion(expectedVersion: Int, actualClass: VirtualFile?) {
requireNotNull(actualClass)
val reader = ClassReader(actualClass.contentsToByteArray())
val node = ClassNode()
reader.accept(node, ClassReader.SKIP_CODE)
assertEquals(
"Expected class version($expectedVersion) differs in ${actualClass.path} (${node.version})",
expectedVersion,
node.version
)
}
lateinit var testRootDisposable: Disposable
@BeforeEach
fun createRootDisposable() {
testRootDisposable = Disposable {}
}
@AfterEach
fun disposeRootDisposable() {
Disposer.dispose(testRootDisposable)
}
/**
* The test ensures that Thread class always contains version from JDK_11, when such javaHome is used
* Regardless of compiler runtime JDK
*/
@Test
fun testClassVersionsInJavaLangOfJdk11() {
val configuration = CompilerConfiguration()
val jdkHome = JvmEnvironmentConfigurator.getJdkHome(TestJdkKind.FULL_JDK_11)
requireNotNull(jdkHome)
configuration.put(JVMConfigurationKeys.JDK_HOME, jdkHome)
val environment =
KotlinCoreEnvironment.getOrCreateApplicationEnvironmentForTests(this.testRootDisposable, configuration)
val jrt = environment.jrtFileSystem ?: error("No jrt-fs configured")
val root = jrt.findFileByPath("$jdkHome!/modules/java.base/java/lang/")
requireNotNull(root)
val children = root.children.filter { it.extension == "class" }
assert(children.isNotEmpty())
children.forEach { file ->
checkClassVersion(
Opcodes.V11,
file
)
}
}
}
@@ -191,6 +191,9 @@ public class KotlinTestUtils {
assert jdk6 != null : "Environment variable JDK_1_6 is not set";
configuration.put(JVMConfigurationKeys.JDK_HOME, new File(jdk6));
}
else if (jdkKind == TestJdkKind.FULL_JDK_11) {
configuration.put(JVMConfigurationKeys.JDK_HOME, KtTestUtil.getJdk11Home());
}
else if (jdkKind == TestJdkKind.FULL_JDK_17) {
configuration.put(JVMConfigurationKeys.JDK_HOME, KtTestUtil.getJdk17Home());
}
@@ -7,6 +7,7 @@
package org.jetbrains.kotlin.codegen.jdk
import org.jetbrains.kotlin.test.jvm.compiler.CoreJrtFsTest
import org.jetbrains.kotlin.test.runners.codegen.*
import org.junit.platform.runner.JUnitPlatform
import org.junit.platform.suite.api.ExcludeTags
@@ -26,7 +27,9 @@ import org.junit.runner.RunWith
IrBlackBoxCodegenTestGenerated::class,
IrBlackBoxInlineCodegenWithBytecodeInlinerTestGenerated::class,
IrCompileKotlinAgainstInlineKotlinTestGenerated::class
IrCompileKotlinAgainstInlineKotlinTestGenerated::class,
CoreJrtFsTest::class
)
@IncludeClassNamePatterns(".*Test.*Generated")
@ExcludeTags("<modernJava>")
@@ -154,6 +154,28 @@ class KotlinDaemonIT : KGPDaemonsBaseTest() {
}
}
@DisplayName("KT-57154: Compiler should use specified toolchain regardless of Gradle Runtime JDK")
@JdkVersions(versions = [JavaVersion.VERSION_1_8, JavaVersion.VERSION_11, JavaVersion.VERSION_17])
@GradleWithJdkTest
@GradleTestVersions(minVersion = TestVersions.Gradle.MAX_SUPPORTED)
internal fun testCompilerRuntimeJdkToolchainIndependence(gradleVersion: GradleVersion, jdkVersion: JdkVersions.ProvidedJdk) {
project(
projectName = "kotlin-java-toolchain/onlyJdk11Compatible",
gradleVersion = gradleVersion,
buildJdk = jdkVersion.location,
buildOptions = defaultBuildOptions.copy(logLevel = LogLevel.INFO)
) {
build("compileKotlin") {
val startOptions = output.findAllStringsPrefixed("starting the daemon as: ").single()
// ensure that new daemon was started and that specified JDK is used as runtime JDK for it
assert(startOptions.startsWith(jdkVersion.location.absolutePath)) {
printBuildOutput()
"Kotlin daemon used non-expected JDK (expected ${jdkVersion.location.absolutePath}): $startOptions"
}
}
}
}
private fun BuildResult.assertGradleClasspathNotLeaked() {
assertOutputContains("Kotlin compiler classpath:")
@@ -0,0 +1,12 @@
plugins {
id "org.jetbrains.kotlin.jvm"
}
repositories {
mavenLocal()
mavenCentral()
}
kotlin {
jvmToolchain(11)
}
@@ -0,0 +1,17 @@
/*
* Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
* Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
*/
/**
* A main function that could only compile on JDK 11-13, due to use of both new api and removed api
*/
fun main() {
// The function, that was only available until JDK 13 (https://bugs.openjdk.org/browse/JDK-8205131)
Runtime.getRuntime().traceInstructions(true)
// The new overload for toArray that was added in JDK 11 (https://bugs.openjdk.org/browse/JDK-8060192)
val array: Array<String> = listOf("").toArray { arrayOf("other") }
}