From b454f34b5991d25d46c3be3c402933bd297e7db2 Mon Sep 17 00:00:00 2001 From: Sergey Shanshin Date: Mon, 6 Feb 2023 17:04:19 +0300 Subject: [PATCH] Prevent class loaders from leaking when using ClassValue cache (#2175) ClassValue can transitively refer to an instance of type java.lang.Class, so this may prevent the loader from being collected during garbage collection. Using SoftReference over the value should solve this problem. --- .../kotlinx/serialization/internal/Caching.kt | 97 ++++++++++++++----- rules/common.pro | 3 +- 2 files changed, 74 insertions(+), 26 deletions(-) diff --git a/core/jvmMain/src/kotlinx/serialization/internal/Caching.kt b/core/jvmMain/src/kotlinx/serialization/internal/Caching.kt index a95d77967..a96f69949 100644 --- a/core/jvmMain/src/kotlinx/serialization/internal/Caching.kt +++ b/core/jvmMain/src/kotlinx/serialization/internal/Caching.kt @@ -5,6 +5,7 @@ package kotlinx.serialization.internal import kotlinx.serialization.KSerializer +import java.lang.ref.SoftReference import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.KClass import kotlin.reflect.KClassifier @@ -40,38 +41,80 @@ internal actual fun createParametrizedCache(factory: (KClass, List(compute: (KClass<*>) -> KSerializer?) : SerializerCache { - private val classValue = ClassValueWrapper(compute) +private class ClassValueCache(val compute: (KClass<*>) -> KSerializer?) : SerializerCache { + private val classValue = ClassValueReferences>() - override fun get(key: KClass): KSerializer? = classValue[key.java].serializer + override fun get(key: KClass): KSerializer? { + return classValue + .getOrSet(key.java) { CacheEntry(compute(key)) } + .serializer + } } +/** + * A class that combines the capabilities of ClassValue and SoftReference. + * Softly binds the calculated value to the specified class. + * + * [SoftReference] used to prevent class loaders from leaking, + * since the value can transitively refer to an instance of type [Class], this may prevent the loader from + * being collected during garbage collection. + * + * In the first calculation the value is cached, every time [getOrSet] is called, a pre-calculated value is returned. + * + * However, the value can be collected during garbage collection (thanks to [SoftReference]) + * - in this case, when trying to call the [getOrSet] function, the value will be calculated again and placed in the cache. + * + * An important requirement for a function generating a value is that it must be stable, so that each time it is called for the same class, the function returns similar values. + * In the case of serializers, these should be instances of the same class filled with equivalent values. + */ @SuppressAnimalSniffer -private class ClassValueWrapper(private val compute: (KClass<*>) -> KSerializer?): ClassValue>() { - /* - * Since during the computing of the value for the `ClassValue` entry, we do not know whether a nullable - * serializer is needed, so we may need to differentiate nullable/non-null caches by a level higher - */ - override fun computeValue(type: Class<*>): CacheEntry { - return CacheEntry(compute(type.kotlin)) +private class ClassValueReferences : ClassValue>() { + override fun computeValue(type: Class<*>): MutableSoftReference { + return MutableSoftReference() } -} -private class ClassValueParametrizedCache(private val compute: (KClass, List) -> KSerializer?) : ParametrizedSerializerCache { - private val classValue = ParametrizedClassValueWrapper() + inline fun getOrSet(key: Class<*>, crossinline factory: () -> T): T { + val ref: MutableSoftReference = get(key) + + ref.reference.get()?.let { return it } + + // go to the slow path and create serializer with blocking, also wrap factory block + return ref.getOrSetWithLock { factory() } + } - override fun get(key: KClass, types: List): Result?> = - classValue[key.java].computeIfAbsent(types) { compute(key, types) } } -@SuppressAnimalSniffer -private class ParametrizedClassValueWrapper : ClassValue>() { +/** + * Wrapper over `SoftReference`, used to store a mutable value. + */ +private class MutableSoftReference { + // volatile because of situations like https://stackoverflow.com/a/7855774 + @JvmField + @Volatile + var reference: SoftReference = SoftReference(null) + /* - * Since during the computing of the value for the `ClassValue` entry, we do not know whether a nullable - * serializer is needed, so we may need to differentiate nullable/non-null caches by a level higher - */ - override fun computeValue(type: Class<*>): ParametrizedCacheEntry { - return ParametrizedCacheEntry() + It is important that the monitor for synchronized is the `MutableSoftReference` of a specific class + This way access to reference is blocked only for one serializable class, and not for all + */ + @Synchronized + fun getOrSetWithLock(factory: () -> T): T { + // exit function if another thread has already filled in the `reference` with non-null value + reference.get()?.let { return it } + + val value = factory() + reference = SoftReference(value) + return value + } +} + +private class ClassValueParametrizedCache(private val compute: (KClass, List) -> KSerializer?) : + ParametrizedSerializerCache { + private val classValue = ClassValueReferences>() + + override fun get(key: KClass, types: List): Result?> { + return classValue.getOrSet(key.java) { ParametrizedCacheEntry() } + .computeIfAbsent(types) { compute(key, types) } } } @@ -91,8 +134,8 @@ private class ConcurrentHashMapCache(private val compute: (KClass<*>) -> KSer } - -private class ConcurrentHashMapParametrizedCache(private val compute: (KClass, List) -> KSerializer?) : ParametrizedSerializerCache { +private class ConcurrentHashMapParametrizedCache(private val compute: (KClass, List) -> KSerializer?) : + ParametrizedSerializerCache { private val cache = ConcurrentHashMap, ParametrizedCacheEntry>() override fun get(key: KClass, types: List): Result?> { @@ -101,6 +144,12 @@ private class ConcurrentHashMapParametrizedCache(private val compute: (KClass } } +/** + * Wrapper for cacheable serializer of some type. + * Used to store cached serializer or indicates that the serializer is not cacheable. + * + * If serializer for type is not cacheable then value of [serializer] is `null`. + */ private class CacheEntry(@JvmField val serializer: KSerializer?) /** diff --git a/rules/common.pro b/rules/common.pro index c70347359..f84d7b979 100644 --- a/rules/common.pro +++ b/rules/common.pro @@ -32,5 +32,4 @@ # Serialization core uses `java.lang.ClassValue` for caching inside these specified classes. # If there is no `java.lang.ClassValue` (for example, in Android), then R8/ProGuard will print a warning. # However, since in this case they will not be used, we can disable these warnings --dontwarn kotlinx.serialization.internal.ClassValueWrapper --dontwarn kotlinx.serialization.internal.ParametrizedClassValueWrapper +-dontwarn kotlinx.serialization.internal.ClassValueReferences