Do not use ClassLoader.getResourceAsStream in reflection

Use ClassLoader.getResource + openStream instead, to workaround an issue
in URLClassLoader.

Also set useCaches to false because kotlin-reflect only reads builtins
metadata once per class loader, and doesn't need it to be cached. Using
caches here might also lead to the problem of closed input streams when
protobuf is read in parallel. The test doesn't check exactly this,
though (it seems to succeed even if cached connections are used).

Note that BuiltInsResourceLoader has a JDK 9+ specialization at
libraries/reflect/api/src/java9, but that implementation does not need
any changes because it uses Module.getResourceAsStream which is not
affected by this issue in URLClassLoader.

 #KT-18277 Fixed
This commit is contained in:
Alexander Udalov
2021-06-02 15:47:22 +02:00
parent a0503aa2d7
commit c1021623e8
2 changed files with 53 additions and 2 deletions
@@ -11,7 +11,12 @@ import org.jetbrains.kotlin.test.KotlinTestUtils
import org.jetbrains.kotlin.test.testFramework.KtUsefulTestCase
import org.jetbrains.kotlin.test.util.KtTestUtil
import java.io.File
import java.net.URLClassLoader
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicReference
import kotlin.concurrent.thread
import kotlin.reflect.KClass
class ReflectionIntegrationTest : KtUsefulTestCase() {
// This test checks that we use the correct class loader to load .kotlin_builtins resource files.
@@ -49,4 +54,45 @@ class ReflectionIntegrationTest : KtUsefulTestCase() {
process.waitFor(1, TimeUnit.MINUTES)
assertEquals(0, process.exitValue())
}
// This test checks that simultaneous access to kotlin-reflect from different threads works, in case the URLClassLoader instance is
// being closed in one of the threads. It creates two threads that for several seconds continuously load kotlin-reflect in a new
// class loader, call something from it, and close the class loader.
fun testParallelAccess() {
val urls = arrayOf(
ForTestCompileRuntime.reflectJarForTests().toURI().toURL(),
ForTestCompileRuntime.runtimeJarForTests().toURI().toURL(),
)
val latch = CountDownLatch(1)
val error = AtomicReference<Throwable?>()
repeat(2) {
thread {
while (latch.count == 1L) {
try {
val classLoader = URLClassLoader(urls, null)
// Invoke KClass.primaryConstructor on String::class reflectively.
// Among other things, it leads to reading a resource kotlin/kotlin.kotlin_builtins from the JAR file.
val getPrimaryConstructor = classLoader.loadClass("kotlin.reflect.full.KClasses")
.getDeclaredMethod("getPrimaryConstructor", classLoader.loadClass(KClass::class.java.name))
val createKClass = classLoader.loadClass("kotlin.jvm.internal.Reflection")
.getDeclaredMethod("getOrCreateKotlinClass", Class::class.java)
getPrimaryConstructor(null, createKClass(null, String::class.java))
// Close URLClassLoader to verify that it won't affect reading the class loader resources from the other thread(s).
classLoader.close()
} catch (e: Throwable) {
error.set(e)
latch.countDown()
}
}
}
}
latch.await(5L, TimeUnit.SECONDS)
latch.countDown()
error.get()?.let { throw it }
}
}
@@ -9,7 +9,12 @@ import java.io.InputStream
class BuiltInsResourceLoader {
fun loadResource(path: String): InputStream? {
return this::class.java.classLoader?.getResourceAsStream(path)
?: ClassLoader.getSystemResourceAsStream(path)
val classLoader = this::class.java.classLoader ?: return ClassLoader.getSystemResourceAsStream(path)
// Do not use getResourceAsStream because URLClassLoader's implementation creates InputStream instances which refer to
// a globally cached JarFile instance, which is closed as soon as URLClassLoader is closed, which instantly invalidates all
// input streams referring to that JarFile and breaks kotlin-reflect in case it's used from different class loaders.
val resource = classLoader.getResource(path) ?: return null
return resource.openConnection().apply { useCaches = false }.getInputStream()
}
}