From 9c1e7f93d2afb45fbc2b0bb743c755866e8f11b0 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 1 Apr 2015 21:20:35 +0300 Subject: [PATCH] Reflection: refactor property construction and fix memory leak Properties obtained by KClass.properties were having strong references to descriptors (captured by closures, which are strongly retained by DescriptorBasedProperty). Support initial value in lazy soft properties --- .../jvm/internal/DescriptorBasedProperty.kt | 23 +++++++++++++++---- .../jvm/internal/KCallableContainerImpl.kt | 9 +++----- .../kotlin/reflect/jvm/internal/KClassImpl.kt | 12 +++++----- .../internal/KMemberExtensionPropertyImpl.kt | 21 ++++++++++------- .../jvm/internal/KMemberPropertyImpl.kt | 18 ++++++++------- .../reflect/jvm/internal/KPackageImpl.kt | 20 +++++++--------- .../KTopLevelExtensionPropertyImpl.kt | 18 +++++++-------- .../jvm/internal/KTopLevelVariableImpl.kt | 15 +++++------- .../jvm/internal/ReflectProperties.java | 13 +++++++++-- 9 files changed, 85 insertions(+), 64 deletions(-) diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt index 8ec141e7f7e..f624b4d7f99 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/DescriptorBasedProperty.kt @@ -18,6 +18,7 @@ package kotlin.reflect.jvm.internal import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.load.java.structure.reflect.desc import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.serialization.ProtoBuf import org.jetbrains.kotlin.serialization.deserialization.NameResolver @@ -26,10 +27,22 @@ import org.jetbrains.kotlin.serialization.jvm.JvmProtoBuf import java.lang.reflect.Field import java.lang.reflect.Method -abstract class DescriptorBasedProperty(computeDescriptor: () -> PropertyDescriptor) { - protected abstract val container: KCallableContainerImpl +abstract class DescriptorBasedProperty private( + container: KCallableContainerImpl, + name: String, + receiverParameterDesc: String?, + descriptorInitialValue: PropertyDescriptor? +) { + constructor(container: KCallableContainerImpl, name: String, receiverParameterClass: Class<*>?) : this( + container, name, receiverParameterClass?.desc, null + ) - protected abstract val name: String + constructor(container: KCallableContainerImpl, descriptor: PropertyDescriptor) : this( + container, + descriptor.getName().asString(), + descriptor.getExtensionReceiverParameter()?.getType()?.let { type -> RuntimeTypeMapper.mapTypeToJvmDesc(type) }, + descriptor + ) private data class PropertyProtoData( val proto: ProtoBuf.Callable, @@ -37,7 +50,9 @@ abstract class DescriptorBasedProperty(computeDescriptor: () -> PropertyDescript val signature: JvmProtoBuf.JvmPropertySignature ) - protected val descriptor: PropertyDescriptor by ReflectProperties.lazySoft(computeDescriptor) + protected val descriptor: PropertyDescriptor by ReflectProperties.lazySoft(descriptorInitialValue) { + container.findPropertyDescriptor(name, receiverParameterDesc) + } // null if this is a property declared in a foreign (Java) class private val protoData: PropertyProtoData? by ReflectProperties.lazyWeak { diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KCallableContainerImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KCallableContainerImpl.kt index 893ceeca3de..ea2231e7778 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KCallableContainerImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KCallableContainerImpl.kt @@ -20,7 +20,6 @@ import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.load.java.structure.reflect.classId import org.jetbrains.kotlin.load.java.structure.reflect.classLoader import org.jetbrains.kotlin.load.java.structure.reflect.createArrayType -import org.jetbrains.kotlin.load.java.structure.reflect.desc import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.scopes.JetScope @@ -42,9 +41,7 @@ abstract class KCallableContainerImpl { abstract val scope: JetScope - protected fun findPropertyDescriptor(name: String, receiverParameterClass: Class<*>? = null): () -> PropertyDescriptor = { - val receiverDesc = receiverParameterClass?.desc - + fun findPropertyDescriptor(name: String, receiverDesc: String? = null): PropertyDescriptor { val properties = scope .getProperties(Name.identifier(name)) .filter { descriptor -> @@ -57,14 +54,14 @@ abstract class KCallableContainerImpl { } if (properties.size() != 1) { - val debugText = if (receiverParameterClass == null) name else "${receiverParameterClass.getSimpleName()}.$name" + val debugText = if (receiverDesc == null) name else "$receiverDesc.$name" throw KotlinReflectionInternalError( if (properties.isEmpty()) "Property '$debugText' not resolved in $this" else "${properties.size()} properties '$debugText' resolved in $this" ) } - properties.single() as PropertyDescriptor + return properties.single() as PropertyDescriptor } // TODO: check resulting method's return type diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt index 6d1a44a6d44..e562fdd3cf6 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KClassImpl.kt @@ -69,14 +69,14 @@ class KClassImpl(override val jClass: Class) : KCallableContainerImpl(), K fun getProperties(declared: Boolean): Collection> = getProperties(extension = false, declared = declared) { descriptor -> - if (descriptor.isVar()) KMutableMemberPropertyImpl(this) { descriptor } - else KMemberPropertyImpl(this) { descriptor } + if (descriptor.isVar()) KMutableMemberPropertyImpl(this, descriptor) + else KMemberPropertyImpl(this, descriptor) } fun getExtensionProperties(declared: Boolean): Collection> = getProperties(extension = true, declared = declared) { descriptor -> - if (descriptor.isVar()) KMutableMemberExtensionPropertyImpl(this) { descriptor } - else KMemberExtensionPropertyImpl(this) { descriptor } + if (descriptor.isVar()) KMutableMemberExtensionPropertyImpl(this, descriptor) + else KMemberExtensionPropertyImpl(this, descriptor) } private fun

> getProperties(extension: Boolean, declared: Boolean, create: (PropertyDescriptor) -> P): Collection

= @@ -91,10 +91,10 @@ class KClassImpl(override val jClass: Class) : KCallableContainerImpl(), K .toList() fun memberProperty(name: String): KMemberProperty = - KMemberPropertyImpl(this, findPropertyDescriptor(name)) + KMemberPropertyImpl(this, name) fun mutableMemberProperty(name: String): KMutableMemberProperty = - KMutableMemberPropertyImpl(this, findPropertyDescriptor(name)) + KMutableMemberPropertyImpl(this, name) override fun equals(other: Any?): Boolean = other is KClassImpl<*> && jClass == other.jClass diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberExtensionPropertyImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberExtensionPropertyImpl.kt index d755d32eb07..e21d2d1f79d 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberExtensionPropertyImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberExtensionPropertyImpl.kt @@ -23,10 +23,11 @@ import kotlin.reflect.IllegalPropertyAccessException import kotlin.reflect.KMemberExtensionProperty import kotlin.reflect.KMutableMemberExtensionProperty -open class KMemberExtensionPropertyImpl( - override val container: KClassImpl, - computeDescriptor: () -> PropertyDescriptor -) : DescriptorBasedProperty(computeDescriptor), KMemberExtensionProperty, KPropertyImpl { +open class KMemberExtensionPropertyImpl : DescriptorBasedProperty, KMemberExtensionProperty, KPropertyImpl { + constructor(container: KClassImpl, name: String, receiverParameterClass: Class) : super(container, name, receiverParameterClass) + + constructor(container: KClassImpl, descriptor: PropertyDescriptor) : super(container, descriptor) + override val name: String get() = descriptor.getName().asString() override val getter: Method get() = super.getter!! @@ -45,10 +46,14 @@ open class KMemberExtensionPropertyImpl( } -class KMutableMemberExtensionPropertyImpl( - container: KClassImpl, - computeDescriptor: () -> PropertyDescriptor -) : KMemberExtensionPropertyImpl(container, computeDescriptor), KMutableMemberExtensionProperty, KMutablePropertyImpl { +class KMutableMemberExtensionPropertyImpl : + KMemberExtensionPropertyImpl, + KMutableMemberExtensionProperty, + KMutablePropertyImpl { + constructor(container: KClassImpl, name: String, receiverParameterClass: Class) : super(container, name, receiverParameterClass) + + constructor(container: KClassImpl, descriptor: PropertyDescriptor) : super(container, descriptor) + override val setter: Method get() = super.setter!! override fun set(instance: D, extensionReceiver: E, value: R) { diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt index ca996a0b7d9..8e7b81071ec 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KMemberPropertyImpl.kt @@ -21,10 +21,11 @@ import kotlin.reflect.IllegalPropertyAccessException import kotlin.reflect.KMemberProperty import kotlin.reflect.KMutableMemberProperty -open class KMemberPropertyImpl( - override val container: KClassImpl, - computeDescriptor: () -> PropertyDescriptor -) : DescriptorBasedProperty(computeDescriptor), KMemberProperty, KPropertyImpl { +open class KMemberPropertyImpl : DescriptorBasedProperty, KMemberProperty, KPropertyImpl { + constructor(container: KClassImpl, name: String) : super(container, name, null) + + constructor(container: KClassImpl, descriptor: PropertyDescriptor) : super(container, descriptor) + override val name: String get() = descriptor.getName().asString() override fun get(instance: T): R { @@ -40,10 +41,11 @@ open class KMemberPropertyImpl( } -class KMutableMemberPropertyImpl( - container: KClassImpl, - computeDescriptor: () -> PropertyDescriptor -) : KMemberPropertyImpl(container, computeDescriptor), KMutableMemberProperty, KMutablePropertyImpl { +class KMutableMemberPropertyImpl : KMemberPropertyImpl, KMutableMemberProperty, KMutablePropertyImpl { + constructor(container: KClassImpl, name: String) : super(container, name) + + constructor(container: KClassImpl, descriptor: PropertyDescriptor) : super(container, descriptor) + override fun set(instance: T, value: R) { try { val setter = setter diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt index dfb085c9684..67d65787724 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt @@ -31,21 +31,17 @@ class KPackageImpl(override val jClass: Class<*>) : KCallableContainerImpl(), KP override val scope: JetScope get() = descriptor.getMemberScope() - fun topLevelVariable(name: String): KTopLevelVariable<*> { - return KTopLevelVariableImpl(this, findPropertyDescriptor(name)) - } + fun topLevelVariable(name: String): KTopLevelVariable<*> = + KTopLevelVariableImpl(this, name) - fun mutableTopLevelVariable(name: String): KMutableTopLevelVariable<*> { - return KMutableTopLevelVariableImpl(this, findPropertyDescriptor(name)) - } + fun mutableTopLevelVariable(name: String): KMutableTopLevelVariable<*> = + KMutableTopLevelVariableImpl(this, name) - fun topLevelExtensionProperty(name: String, receiver: Class): KTopLevelExtensionProperty { - return KTopLevelExtensionPropertyImpl(this, findPropertyDescriptor(name, receiver)) - } + fun topLevelExtensionProperty(name: String, receiver: Class): KTopLevelExtensionProperty = + KTopLevelExtensionPropertyImpl(this, name, receiver) - fun mutableTopLevelExtensionProperty(name: String, receiver: Class): KMutableTopLevelExtensionProperty { - return KMutableTopLevelExtensionPropertyImpl(this, findPropertyDescriptor(name, receiver)) - } + fun mutableTopLevelExtensionProperty(name: String, receiver: Class): KMutableTopLevelExtensionProperty = + KMutableTopLevelExtensionPropertyImpl(this, name, receiver) override fun equals(other: Any?): Boolean = other is KPackageImpl && jClass == other.jClass diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelExtensionPropertyImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelExtensionPropertyImpl.kt index 5e08b1b1080..209efd23d3d 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelExtensionPropertyImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelExtensionPropertyImpl.kt @@ -16,16 +16,14 @@ package kotlin.reflect.jvm.internal -import org.jetbrains.kotlin.descriptors.PropertyDescriptor import java.lang.reflect.Method import kotlin.reflect.IllegalPropertyAccessException import kotlin.reflect.KMutableTopLevelExtensionProperty import kotlin.reflect.KTopLevelExtensionProperty -open class KTopLevelExtensionPropertyImpl( - override val container: KPackageImpl, - computeDescriptor: () -> PropertyDescriptor -) : DescriptorBasedProperty(computeDescriptor), KTopLevelExtensionProperty, KPropertyImpl { +open class KTopLevelExtensionPropertyImpl : DescriptorBasedProperty, KTopLevelExtensionProperty, KPropertyImpl { + constructor(container: KPackageImpl, name: String, receiverParameterClass: Class) : super(container, name, receiverParameterClass) + override val name: String get() = descriptor.getName().asString() override val getter: Method get() = super.getter!! @@ -41,10 +39,12 @@ open class KTopLevelExtensionPropertyImpl( } } -class KMutableTopLevelExtensionPropertyImpl( - container: KPackageImpl, - computeDescriptor: () -> PropertyDescriptor -) : KTopLevelExtensionPropertyImpl(container, computeDescriptor), KMutableTopLevelExtensionProperty, KMutablePropertyImpl { +class KMutableTopLevelExtensionPropertyImpl : + KTopLevelExtensionPropertyImpl, + KMutableTopLevelExtensionProperty, + KMutablePropertyImpl { + constructor(container: KPackageImpl, name: String, receiverParameterClass: Class) : super(container, name, receiverParameterClass) + override val setter: Method get() = super.setter!! override fun set(receiver: T, value: R) { diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelVariableImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelVariableImpl.kt index 524c5308246..36b07b671e1 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelVariableImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KTopLevelVariableImpl.kt @@ -16,16 +16,14 @@ package kotlin.reflect.jvm.internal -import org.jetbrains.kotlin.descriptors.PropertyDescriptor import java.lang.reflect.Method import kotlin.reflect.IllegalPropertyAccessException import kotlin.reflect.KMutableTopLevelVariable import kotlin.reflect.KTopLevelVariable -open class KTopLevelVariableImpl( - override val container: KPackageImpl, - computeDescriptor: () -> PropertyDescriptor -) : DescriptorBasedProperty(computeDescriptor), KTopLevelVariable, KVariableImpl { +open class KTopLevelVariableImpl : DescriptorBasedProperty, KTopLevelVariable, KVariableImpl { + constructor(container: KPackageImpl, name: String) : super(container, name, null) + override val name: String get() = descriptor.getName().asString() override val getter: Method get() = super.getter!! @@ -41,10 +39,9 @@ open class KTopLevelVariableImpl( } } -class KMutableTopLevelVariableImpl( - container: KPackageImpl, - computeDescriptor: () -> PropertyDescriptor -) : KTopLevelVariableImpl(container, computeDescriptor), KMutableTopLevelVariable, KMutableVariableImpl { +class KMutableTopLevelVariableImpl : KTopLevelVariableImpl, KMutableTopLevelVariable, KMutableVariableImpl { + constructor(container: KPackageImpl, name: String) : super(container, name) + override val setter: Method get() = super.setter!! override fun set(value: R) { diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/ReflectProperties.java b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/ReflectProperties.java index 25d6cf86f75..df36559c784 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/ReflectProperties.java +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/ReflectProperties.java @@ -18,6 +18,7 @@ package kotlin.reflect.jvm.internal; import kotlin.Function0; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.lang.ref.SoftReference; import java.lang.ref.WeakReference; @@ -67,8 +68,11 @@ import java.lang.ref.WeakReference; private final Function0 initializer; private SoftReference value = null; - public LazySoftVal(@NotNull Function0 initializer) { + public LazySoftVal(@Nullable T initialValue, @NotNull Function0 initializer) { this.initializer = initializer; + if (initialValue != null) { + this.value = new SoftReference(escape(initialValue)); + } } @Override @@ -120,9 +124,14 @@ import java.lang.ref.WeakReference; return new LazyVal(initializer); } + @NotNull + public static LazySoftVal lazySoft(@Nullable T initialValue, @NotNull Function0 initializer) { + return new LazySoftVal(initialValue, initializer); + } + @NotNull public static LazySoftVal lazySoft(@NotNull Function0 initializer) { - return new LazySoftVal(initializer); + return lazySoft(null, initializer); } @NotNull