From c9bde57e4413017c6e86a5c4c2c09169d3ff1cad Mon Sep 17 00:00:00 2001 From: "Sergey.Shanshin" Date: Tue, 9 May 2023 09:59:11 +0000 Subject: [PATCH] [KxSerialization] Fix NPE if sealed class has self-referencing property Fixes Kotlin/kotlinx.serialization#2294 If the serializable class contains itself as a property, and its serializer is not an object (for example, for a sealed class), in this case there is a race in the initialization of the serializer and child serializers. To avoid this the serialization class should not cache its own serializer as a child. Merge-request: KT-MR-10019 Merged-by: Sergey Shanshin --- .../compiler/backend/ir/BaseIrGenerator.kt | 14 +++++++-- .../backend/ir/SerializableIrGenerator.kt | 2 +- .../backend/ir/SerializerIrGenerator.kt | 2 +- .../testData/boxIr/caching.kt | 29 +++++++++++++++++++ ...tionFirLightTreeBlackBoxTestGenerated.java | 6 ++++ .../SerializationIrBoxTestGenerated.java | 6 ++++ 6 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 plugins/kotlinx-serialization/testData/boxIr/caching.kt diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/BaseIrGenerator.kt b/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/BaseIrGenerator.kt index 5accdd4fe6f..05c12df137e 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/BaseIrGenerator.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/BaseIrGenerator.kt @@ -429,14 +429,24 @@ abstract class BaseIrGenerator(private val currentClass: IrClass, final override } fun IrClass.createCachedChildSerializers( + serializableClass: IrClass, serializableProperties: List ): List { return DeclarationIrBuilder(compilerContext, symbol).run { - serializableProperties.map { cacheableChildSerializerInstance(it) } + serializableProperties.map { cacheableChildSerializerInstance(serializableClass, it) } } } - private fun IrBuilderWithScope.cacheableChildSerializerInstance(property: IrSerializableProperty): IrExpression? { + private fun IrBuilderWithScope.cacheableChildSerializerInstance( + serializableClass: IrClass, + property: IrSerializableProperty + ): IrExpression? { + // to avoid a cyclical dependency between the serializer cache and the cache of child serializers, + // the class should not cache its serializer as a child + if (serializableClass.symbol == property.type.classifier) { + return null + } + val serializer = getIrSerialTypeInfo(property, compilerContext).serializer ?: return null if (serializer.owner.kind == ClassKind.OBJECT) return null diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt b/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt index 99c0cff7dc8..0d17082b5bc 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializableIrGenerator.kt @@ -55,7 +55,7 @@ class SerializableIrGenerator( private val IrClass.isInternalSerializable: Boolean get() = kind == ClassKind.CLASS && hasSerializableOrMetaAnnotationWithoutArgs() private val cachedChildSerializers: List = - irClass.companionObject()!!.createCachedChildSerializers(properties.serializableProperties) + irClass.companionObject()!!.createCachedChildSerializers(irClass, properties.serializableProperties) private val cachedChildSerializersProperty: IrProperty? = irClass.companionObject()!!.addCachedChildSerializersProperty(cachedChildSerializers) diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt b/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt index e4d92d964f3..c20524b8a03 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.backend/src/org/jetbrains/kotlinx/serialization/compiler/backend/ir/SerializerIrGenerator.kt @@ -89,7 +89,7 @@ open class SerializerIrGenerator( // non-object serializers which can be cached private val cacheableChildSerializers by lazy { - serializableIrClass.createCachedChildSerializers(properties.serializableProperties).map { it != null } + serializableIrClass.createCachedChildSerializers(serializableIrClass, properties.serializableProperties).map { it != null } } // null if was not found — we're in FIR diff --git a/plugins/kotlinx-serialization/testData/boxIr/caching.kt b/plugins/kotlinx-serialization/testData/boxIr/caching.kt new file mode 100644 index 00000000000..19b04ed5f9a --- /dev/null +++ b/plugins/kotlinx-serialization/testData/boxIr/caching.kt @@ -0,0 +1,29 @@ +// TARGET_BACKEND: JVM_IR + +// WITH_STDLIB + +import kotlinx.serialization.* + +// Serializers for classes containing themselves as properties should be cached correctly +@Serializable +class Plain(val p: Plain? = null) + +@Serializable +sealed class Sealed(val p: Sealed? = null) + +@Serializable +open class Open(val p: Open? = null) + +@Serializable +abstract class Abstract(val p: Abstract? = null) + + +fun box(): String { + // A correctly cached class must be initialized correctly in order to exclude cyclic nesting of caches + Plain.serializer() + Sealed.serializer() + Open.serializer() + Abstract.serializer() + + return "OK" +} diff --git a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirLightTreeBlackBoxTestGenerated.java b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirLightTreeBlackBoxTestGenerated.java index 6ec2466bd18..ba153327a3f 100644 --- a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirLightTreeBlackBoxTestGenerated.java +++ b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirLightTreeBlackBoxTestGenerated.java @@ -33,6 +33,12 @@ public class SerializationFirLightTreeBlackBoxTestGenerated extends AbstractSeri runTest("plugins/kotlinx-serialization/testData/boxIr/annotationsOnFile.kt"); } + @Test + @TestMetadata("caching.kt") + public void testCaching() throws Exception { + runTest("plugins/kotlinx-serialization/testData/boxIr/caching.kt"); + } + @Test @TestMetadata("classSerializerAsObject.kt") public void testClassSerializerAsObject() throws Exception { diff --git a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrBoxTestGenerated.java b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrBoxTestGenerated.java index 0603fc9c26d..57ade1a1ef7 100644 --- a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrBoxTestGenerated.java +++ b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationIrBoxTestGenerated.java @@ -31,6 +31,12 @@ public class SerializationIrBoxTestGenerated extends AbstractSerializationIrBoxT runTest("plugins/kotlinx-serialization/testData/boxIr/annotationsOnFile.kt"); } + @Test + @TestMetadata("caching.kt") + public void testCaching() throws Exception { + runTest("plugins/kotlinx-serialization/testData/boxIr/caching.kt"); + } + @Test @TestMetadata("classSerializerAsObject.kt") public void testClassSerializerAsObject() throws Exception {