From 79f6d4b590573e6adccd7e8899d3b15ddb42d185 Mon Sep 17 00:00:00 2001
From: Vsevolod Tolstopyatov <qwwdfsad@gmail.com>
Date: Fri, 27 Jan 2023 17:01:21 +0100
Subject: [PATCH] Address classloader leak in a recently-introduced
 ClassValueCache

Root cause:
Cached in ClassValue values are prohibited to reference any java.lang.Class or ClassValue instances as they are considered as strong roots that prevent Class and ClassValue garbage collection,
creating effectively unloadable cycle.
This problem is also known as JDK-8136353.

Actions taken:
* Extract anonymous ClassValue instance into a separate static class that does not capture anything implicitly
* Wrap cached values in ClassValue into SoftReference to avoid unloadable cycles

^KT-56093 Fixed
---
 .../reflect/jvm/internal/CacheByClass.kt      | 46 ++++++++++++++-----
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/CacheByClass.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/CacheByClass.kt
index f95e0cc5a0b..b8330af62e9 100644
--- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/CacheByClass.kt
+++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/CacheByClass.kt
@@ -5,6 +5,7 @@
 
 package kotlin.reflect.jvm.internal
 
+import java.lang.ref.SoftReference
 import java.util.concurrent.ConcurrentHashMap
 
 /*
@@ -26,7 +27,7 @@ internal abstract class CacheByClass<V> {
 }
 
 /**
- * Creates a **strongly referenced** cache of values associated with [Class].
+ * Creates a **softly referenced** cache of values associated with [Class].
  * Values are computed using provided [compute] function.
  *
  * `null` values are not supported, though there aren't any technical limitations.
@@ -35,25 +36,46 @@ internal fun <V : Any> createCache(compute: (Class<*>) -> V): CacheByClass<V> {
     return if (useClassValue) ClassValueCache(compute) else ConcurrentHashMapCache(compute)
 }
 
-private class ClassValueCache<V>(private val compute: (Class<*>) -> V) : CacheByClass<V>() {
-
-    @Volatile
-    private var classValue = initClassValue()
-
-    private fun initClassValue() = object : ClassValue<V>() {
-        override fun computeValue(type: Class<*>): V {
-            return compute(type)
-        }
+/*
+ * We can only cache SoftReference instances in our own classvalue to avoid classloader-based leak.
+ *
+ * In short, the following uncollectable cycle is possible otherwise:
+ * ClassValue -> KPackageImpl.getClass() -> UrlClassloader -> all loaded classes by this CL ->
+ *  -> kotlin.reflect.jvm.internal.ClassValueCache -> ClassValue
+ */
+private class ComputableClassValue<V>(@JvmField val compute: (Class<*>) -> V) : ClassValue<SoftReference<V>>() {
+    override fun computeValue(type: Class<*>): SoftReference<V> {
+        return SoftReference(compute(type))
     }
 
-    override fun get(key: Class<*>): V = classValue[key]
+    fun createNewCopy() = ComputableClassValue(compute)
+}
+
+private class ClassValueCache<V>(compute: (Class<*>) -> V) : CacheByClass<V>() {
+
+    @Volatile
+    private var classValue = ComputableClassValue(compute)
+
+    override fun get(key: Class<*>): V {
+        val classValue = classValue
+        classValue[key].get()?.let { return it }
+        // Clean stale value if it was collected at some point
+        classValue.remove(key)
+        /*
+         * Optimistic assumption: the value was invalidated at some point of time,
+         * but now we do not have a memory pressure and can recompute the value
+         */
+        classValue[key].get()?.let { return it }
+        // Assumption failed, do not retry to avoid any non-trivial GC-dependent loops and deliberately create a separate copy
+        return classValue.compute(key)
+    }
 
     override fun clear() {
         /*
          * ClassValue does not have a proper `clear()` method but is properly weak-referenced,
          * thus abandoning ClassValue instance will eventually clear all associated values.
          */
-        classValue = initClassValue()
+        classValue = classValue.createNewCopy()
     }
 }