From c1021623e87fdae7b6160a81510f631fb6352dbe Mon Sep 17 00:00:00 2001
From: Alexander Udalov <alexander.udalov@jetbrains.com>
Date: Wed, 2 Jun 2021 15:47:22 +0200
Subject: [PATCH] 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
---
 .../reflection/ReflectionIntegrationTest.kt   | 46 +++++++++++++++++++
 .../builtins/BuiltInsResourceLoader.kt        |  9 +++-
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/compiler/tests/org/jetbrains/kotlin/reflection/ReflectionIntegrationTest.kt b/compiler/tests/org/jetbrains/kotlin/reflection/ReflectionIntegrationTest.kt
index 5a6fade3c05..cdab34b7a1d 100644
--- a/compiler/tests/org/jetbrains/kotlin/reflection/ReflectionIntegrationTest.kt
+++ b/compiler/tests/org/jetbrains/kotlin/reflection/ReflectionIntegrationTest.kt
@@ -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 }
+    }
 }
diff --git a/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/builtins/BuiltInsResourceLoader.kt b/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/builtins/BuiltInsResourceLoader.kt
index c0e49affc11..1fa734fb290 100644
--- a/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/builtins/BuiltInsResourceLoader.kt
+++ b/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/builtins/BuiltInsResourceLoader.kt
@@ -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()
     }
 }