From 2934588bbed99b7dbb4c8949c6d580da3d9cdf49 Mon Sep 17 00:00:00 2001 From: Svyatoslav Scherbina Date: Mon, 4 Jun 2018 15:34:35 +0300 Subject: [PATCH] Fix garbage cycle collection for kotlin.Array --- .../kotlin/backend/konan/llvm/RTTIGenerator.kt | 8 +++++++- backend.native/tests/build.gradle | 4 ++++ backend.native/tests/runtime/memory/cycles1.kt | 17 +++++++++++++++++ runtime/src/main/cpp/Memory.cpp | 13 +++++++------ runtime/src/main/cpp/TypeInfo.h | 4 +++- 5 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 backend.native/tests/runtime/memory/cycles1.kt diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt index 7a6d6cf0278..6f2c038b14c 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt @@ -165,6 +165,12 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { val objOffsetsPtr = staticData.placeGlobalConstArray("krefs:$className", int32Type, objOffsets.map { Int32(it.toInt()) }) + val objOffsetsCount = if (classDesc.descriptor == context.builtIns.array) { + 1 // To mark it as non-leaf. + } else { + objOffsets.size + } + val fields = llvmDeclarations.fields.mapIndexed { index, field -> // Note: using FQ name because a class may have multiple fields with the same name due to property overriding val nameSignature = field.fqNameSafe.localHash // FIXME: add signature @@ -191,7 +197,7 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { name, size, superType, - objOffsetsPtr, objOffsets.size, + objOffsetsPtr, objOffsetsCount, interfacesPtr, interfaces.size, methodsPtr, methods.size, fieldsPtr, if (classDesc.isInterface) -1 else fields.size, diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 9eb0af6f2f1..f6409f86a85 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -2117,6 +2117,10 @@ task memory_cycles0(type: RunKonanTest) { source = "runtime/memory/cycles0.kt" } +task memory_cycles1(type: RunKonanTest) { + source = "runtime/memory/cycles1.kt" +} + task memory_basic0(type: RunKonanTest) { source = "runtime/memory/basic0.kt" } diff --git a/backend.native/tests/runtime/memory/cycles1.kt b/backend.native/tests/runtime/memory/cycles1.kt new file mode 100644 index 00000000000..a6cdae98300 --- /dev/null +++ b/backend.native/tests/runtime/memory/cycles1.kt @@ -0,0 +1,17 @@ +package runtime.memory.cycles1 + +import kotlin.test.* +import konan.ref.* + +@Test fun runTest() { + val weakRefToTrashCycle = createLoop() + konan.internal.GC.collect() + assertNull(weakRefToTrashCycle.get()) +} + +private fun createLoop(): WeakReference { + val loop = Array(1, { null }) + loop[0] = loop + + return WeakReference(loop) +} \ No newline at end of file diff --git a/runtime/src/main/cpp/Memory.cpp b/runtime/src/main/cpp/Memory.cpp index ba0188bb2ee..4418ef03539 100644 --- a/runtime/src/main/cpp/Memory.cpp +++ b/runtime/src/main/cpp/Memory.cpp @@ -442,12 +442,13 @@ inline void traverseContainerObjectFields(ContainerHeader* container, func proce ObjHeader* obj = reinterpret_cast(container + 1); for (int object = 0; object < container->objectCount(); object++) { const TypeInfo* typeInfo = obj->type_info(); - for (int index = 0; index < typeInfo->objOffsetsCount_; index++) { - ObjHeader** location = reinterpret_cast( - reinterpret_cast(obj + 1) + typeInfo->objOffsets_[index]); - process(location); - } - if (typeInfo == theArrayTypeInfo) { + if (typeInfo != theArrayTypeInfo) { + for (int index = 0; index < typeInfo->objOffsetsCount_; index++) { + ObjHeader** location = reinterpret_cast( + reinterpret_cast(obj + 1) + typeInfo->objOffsets_[index]); + process(location); + } + } else { ArrayHeader* array = obj->array(); for (int index = 0; index < array->count_; index++) { process(ArrayAddressOfElementAt(array, index)); diff --git a/runtime/src/main/cpp/TypeInfo.h b/runtime/src/main/cpp/TypeInfo.h index e3142518a40..d280ec2a14f 100644 --- a/runtime/src/main/cpp/TypeInfo.h +++ b/runtime/src/main/cpp/TypeInfo.h @@ -81,8 +81,10 @@ struct TypeInfo { int32_t instanceSize_; // Must be pointer to Any for array classes, and null for Any. const TypeInfo* superType_; - // All object references inside this object. + // All object reference fields inside this object. const int32_t* objOffsets_; + // Count of object reference fields inside this object. + // 1 for kotlin.Array to mark it as non-leaf. int32_t objOffsetsCount_; const TypeInfo* const* implementedInterfaces_; int32_t implementedInterfacesCount_;