Skip to content

Commit

Permalink
Address classloader leak in a recently-introduced ClassValueCache
Browse files Browse the repository at this point in the history
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
  • Loading branch information
qwwdfsad authored and qodana-bot committed Jan 30, 2023
1 parent 46ddcac commit 08dd52f
Showing 1 changed file with 32 additions and 10 deletions.
42 changes: 32 additions & 10 deletions core/reflection.jvm/src/kotlin/reflect/jvm/internal/CacheByClass.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package kotlin.reflect.jvm.internal

import java.lang.ref.SoftReference
import java.util.concurrent.ConcurrentHashMap

/*
Expand All @@ -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.
Expand All @@ -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>() {
/*
* 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))
}

fun createNewCopy() = ComputableClassValue(compute)
}

private class ClassValueCache<V>(compute: (Class<*>) -> V) : CacheByClass<V>() {

@Volatile
private var classValue = initClassValue()
private var classValue = ComputableClassValue(compute)

private fun initClassValue() = object : ClassValue<V>() {
override fun computeValue(type: Class<*>): V {
return compute(type)
}
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 get(key: Class<*>): V = classValue[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()
}
}

Expand Down

0 comments on commit 08dd52f

Please sign in to comment.