Introduce ClassValue-based exception constructor cache to stracktrace recovery (#2997)

* Use ClassValue-based cache for exception constructors with stacktrace recovery
* It eliminates the classloader leak in containerized environments
* Insignificantly improves the performance of exception copying
* Creates a precedent of guarded use of non-Android-compliant API


Co-authored-by: dkhalanskyjb <[email protected]>

Author: qwwdfsad

kotlinx-coroutines-core/build.gradle

@@ -201,12 +201,6 @@ task checkJdk16() {
     }
 }
 
-tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile) {
-    kotlinOptions.jdkHome = System.env.JDK_16
-    // only fail when actually trying to compile, not during project setup phase
-    dependsOn(checkJdk16)
-}
-
 jvmTest {
     minHeapSize = '1g'
     maxHeapSize = '1g'
@@ -283,7 +277,6 @@ task jdk16Test(type: Test, dependsOn: [compileTestKotlinJvm, checkJdk16]) {
     classpath = files { jvmTest.classpath }
     testClassesDirs = files { jvmTest.testClassesDirs }
     executable = "$System.env.JDK_16/bin/java"
-    exclude '**/*LFStressTest.*' // lock-freedom tests use LockFreedomTestEnvironment which needs JDK8
     exclude '**/*LincheckTest.*' // Lincheck tests use LinChecker which needs JDK8
     exclude '**/exceptions/**'   // exceptions tests check suppressed exception which needs JDK8
     exclude '**/ExceptionsGuideTest.*'

kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstructor.kt

@@ -11,44 +11,42 @@ import java.util.concurrent.locks.*
 import kotlin.concurrent.*
 
 private val throwableFields = Throwable::class.java.fieldsCountOrDefault(-1)
-private val cacheLock = ReentrantReadWriteLock()
 private typealias Ctor = (Throwable) -> Throwable?
-// Replace it with ClassValue when Java 6 support is over
-private val exceptionCtors: WeakHashMap<Class<out Throwable>, Ctor> = WeakHashMap()
+
+private val ctorCache = try {
+    if (ANDROID_DETECTED) WeakMapCtorCache
+    else ClassValueCtorCache
+} catch (e: Throwable) {
+    // Fallback on Java 6 or exotic setups
+    WeakMapCtorCache
+}
 
 @Suppress("UNCHECKED_CAST")
 internal fun <E : Throwable> tryCopyException(exception: E): E? {
     // Fast path for CopyableThrowable
     if (exception is CopyableThrowable<*>) {
         return runCatching { exception.createCopy() as E? }.getOrNull()
     }
-    // Use cached ctor if found
-    cacheLock.read { exceptionCtors[exception.javaClass] }?.let { cachedCtor ->
-        return cachedCtor(exception) as E?
-    }
-    /*
-     * Skip reflective copy if an exception has additional fields (that are usually populated in user-defined constructors)
-     */
-    if (throwableFields != exception.javaClass.fieldsCountOrDefault(0)) {
-        cacheLock.write { exceptionCtors[exception.javaClass] = { null } }
-        return null
-    }
+    return ctorCache.get(exception.javaClass).invoke(exception) as E?
+}
+
+private fun <E : Throwable> createConstructor(clz: Class<E>): Ctor {
+    val nullResult: Ctor = { null } // Pre-cache class
+    // Skip reflective copy if an exception has additional fields (that are usually populated in user-defined constructors)
+    if (throwableFields != clz.fieldsCountOrDefault(0)) return nullResult
     /*
-     * Try to reflectively find constructor(), constructor(message, cause), constructor(cause) or constructor(message).
-     * Exceptions are shared among coroutines, so we should copy exception before recovering current stacktrace.
-     */
-    var ctor: Ctor? = null
-    val constructors = exception.javaClass.constructors.sortedByDescending { it.parameterTypes.size }
+    * Try to reflectively find constructor(), constructor(message, cause), constructor(cause) or constructor(message).
+    * Exceptions are shared among coroutines, so we should copy exception before recovering current stacktrace.
+    */
+    val constructors = clz.constructors.sortedByDescending { it.parameterTypes.size }
     for (constructor in constructors) {
-        ctor = createConstructor(constructor)
-        if (ctor != null) break
+        val result = createSafeConstructor(constructor)
+        if (result != null) return result
     }
-    // Store the resulting ctor to cache
-    cacheLock.write { exceptionCtors[exception.javaClass] = ctor ?: { null } }
-    return ctor?.invoke(exception) as E?
+    return nullResult
 }
 
-private fun createConstructor(constructor: Constructor<*>): Ctor? {
+private fun createSafeConstructor(constructor: Constructor<*>): Ctor? {
     val p = constructor.parameterTypes
     return when (p.size) {
         2 -> when {
@@ -71,11 +69,41 @@ private fun createConstructor(constructor: Constructor<*>): Ctor? {
 private inline fun safeCtor(crossinline block: (Throwable) -> Throwable): Ctor =
     { e -> runCatching { block(e) }.getOrNull() }
 
-private fun Class<*>.fieldsCountOrDefault(defaultValue: Int) = kotlin.runCatching { fieldsCount() }.getOrDefault(defaultValue)
+private fun Class<*>.fieldsCountOrDefault(defaultValue: Int) =
+    kotlin.runCatching { fieldsCount() }.getOrDefault(defaultValue)
 
 private tailrec fun Class<*>.fieldsCount(accumulator: Int = 0): Int {
     val fieldsCount = declaredFields.count { !Modifier.isStatic(it.modifiers) }
     val totalFields = accumulator + fieldsCount
     val superClass = superclass ?: return totalFields
     return superClass.fieldsCount(totalFields)
 }
+
+internal abstract class CtorCache {
+    abstract fun get(key: Class<out Throwable>): Ctor
+}
+
+private object WeakMapCtorCache : CtorCache() {
+    private val cacheLock = ReentrantReadWriteLock()
+    private val exceptionCtors: WeakHashMap<Class<out Throwable>, Ctor> = WeakHashMap()
+
+    override fun get(key: Class<out Throwable>): Ctor {
+        cacheLock.read { exceptionCtors[key]?.let { return it } }
+        cacheLock.write {
+            exceptionCtors[key]?.let { return it }
+            return createConstructor(key).also { exceptionCtors[key] = it }
+        }
+    }
+}
+
+@IgnoreJreRequirement
+private object ClassValueCtorCache : CtorCache() {
+    private val cache = object : ClassValue<Ctor>() {
+        override fun computeValue(type: Class<*>?): Ctor {
+            @Suppress("UNCHECKED_CAST")
+            return createConstructor(type as Class<out Throwable>)
+        }
+    }
+
+    override fun get(key: Class<out Throwable>): Ctor = cache.get(key)
+}

ui/kotlinx-coroutines-android/test/R8ServiceLoaderOptimizationTest.kt

@@ -11,7 +11,6 @@ import java.io.*
 import java.util.stream.*
 import kotlin.test.*
 
-@Ignore
 class R8ServiceLoaderOptimizationTest : TestBase() {
     private val r8Dex = File(System.getProperty("dexPath")!!).asDexFile()
     private val r8DexNoOptim = File(System.getProperty("noOptimDexPath")!!).asDexFile()