From aae8441b6ee4f394cdcc0184175b81f46dee45bd Mon Sep 17 00:00:00 2001 From: Nikolay Igotti Date: Tue, 4 Feb 2020 17:09:53 +0300 Subject: [PATCH] Remove obsolete leak detector. (#3819) --- backend.native/tests/build.gradle | 6 - .../tests/runtime/memory/leak_detector.kt | 77 ---------- runtime/src/main/cpp/Memory.cpp | 139 +----------------- runtime/src/main/cpp/Memory.h | 7 - .../kotlin/native/concurrent/Atomics.kt | 8 +- .../main/kotlin/kotlin/native/internal/GC.kt | 18 --- 6 files changed, 4 insertions(+), 251 deletions(-) delete mode 100644 backend.native/tests/runtime/memory/leak_detector.kt diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 221faebb4a4..936597d7912 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -2729,12 +2729,6 @@ standaloneTest("memory_only_gc") { source = "runtime/memory/only_gc.kt" } -standaloneTest("leak_detector") { - disabled = project.globalTestArgs.contains('-opt') || (project.testTarget == 'wasm32') // Needs debug build. - flags = ['-g'] - source = "runtime/memory/leak_detector.kt" -} - standaloneTest("cycle_collector") { disabled = project.globalTestArgs.contains('-opt') || (project.testTarget == 'wasm32') // Needs debug build. flags = ['-g'] diff --git a/backend.native/tests/runtime/memory/leak_detector.kt b/backend.native/tests/runtime/memory/leak_detector.kt deleted file mode 100644 index b0d2cd8939d..00000000000 --- a/backend.native/tests/runtime/memory/leak_detector.kt +++ /dev/null @@ -1,77 +0,0 @@ -import kotlin.native.concurrent.* -import kotlin.native.internal.GC -import kotlin.test.* - -/* - * Typical snippet for the leak detector usage. - */ -fun dumpLeaks() { - GC.collect() - GC.detectCycles()?.let { cycles -> - cycles.firstOrNull()?.let { root -> - val cycle = GC.findCycle(root) - println(cycle?.contentToString()) - } - } -} - -fun test1() { - val a = AtomicReference(null) - val b = AtomicReference(null) - a.value = b - b.value = a - val cycles = GC.detectCycles()!! - assertEquals(1, cycles.size) - val cycle = GC.findCycle(cycles[0])!! - assertEquals(2, cycle.size) - assertTrue(cycle.contains(a)) - assertTrue(cycle.contains(b)) - a.value = null -} - -class Holder(var other: Any?) - -fun test2() { - val array = arrayOf(AtomicReference(null), AtomicReference(null)) - val obj1 = Holder(array).freeze() - array[0].value = obj1 - val cycles = GC.detectCycles()!! - assertEquals(1, cycles.size) - assertTrue(arrayOf(obj1, array, array[0]).contentEquals(GC.findCycle(cycles[0])!!)) - array[0].value = null -} - -fun test3() { - val a1 = FreezableAtomicReference(null) - val head = Holder(null) - var current = head - repeat(30) { - val next = Holder(null) - current.other = next - current = next - } - a1.value = head - current.other = a1 - current.freeze() - val cycles = GC.detectCycles()!! - assertEquals(1, cycles.size) - val cycle = GC.findCycle(cycles[0])!! - assertEquals(32, cycle.size) - a1.value = null -} - - -fun test4() { - val atomic = AtomicReference(null) - atomic.value = Pair(atomic, Holder(atomic)).freeze() -} - -fun main() { - // We must disable cyclic collector here, to avoid interfering with cycle detector. - kotlin.native.internal.GC.cyclicCollectorEnabled = false - /*test1() - test2() - test3() */ - test4() - kotlin.native.internal.GC.cyclicCollectorEnabled = true -} \ No newline at end of file diff --git a/runtime/src/main/cpp/Memory.cpp b/runtime/src/main/cpp/Memory.cpp index b9e436f0fcf..bc55fb5c80b 100644 --- a/runtime/src/main/cpp/Memory.cpp +++ b/runtime/src/main/cpp/Memory.cpp @@ -115,12 +115,7 @@ volatile int allocCount = 0; volatile int aliveMemoryStatesCount = 0; KBoolean g_checkLeaks = KonanNeedDebugInfo; - -// Only used by the leak detector. -KRef g_leakCheckerGlobalList = nullptr; -KInt g_leakCheckerGlobalLock = 0; - -bool g_hasCyclicCollector = true; +KBoolean g_hasCyclicCollector = true; // TODO: can we pass this variable as an explicit argument? THREAD_LOCAL_VARIABLE MemoryState* memoryState = nullptr; @@ -940,24 +935,6 @@ void runDeallocationHooks(ContainerHeader* container) { } #endif // USE_CYCLIC_GC if (obj->has_meta_object()) { - if (KonanNeedDebugInfo && (obj->type_info()->flags_ & TF_LEAK_DETECTOR_CANDIDATE) != 0 && g_checkLeaks) { - // Remove the object from the double-linked list of potentially cyclic objects. - auto* meta = obj->meta_object(); - lock(&g_leakCheckerGlobalLock); - // Get previous. - auto* previous = meta->LeakDetector.previous_; - auto* previousMeta = (previous != nullptr) ? previous->meta_object() : nullptr; - auto* next = meta->LeakDetector.next_; - auto* nextMeta = (next != nullptr) ? next->meta_object() : nullptr; - // Remove current. - if (previous != nullptr) - previous->meta_object()->LeakDetector.next_ = next; - if (next != nullptr) - next->meta_object()->LeakDetector.previous_ = previous; - if (obj == g_leakCheckerGlobalList) - g_leakCheckerGlobalList = next; - unlock(&g_leakCheckerGlobalLock); - } ObjHeader::destroyMetaObject(&obj->typeInfoOrMeta_); } obj = reinterpret_cast(reinterpret_cast(obj) + objectSize(obj)); @@ -1954,17 +1931,6 @@ OBJ_GETTER(allocInstance, const TypeInfo* type_info) { #endif // USE_GC auto container = ObjectContainer(state, type_info); ObjHeader* obj = container.GetPlace(); - if (KonanNeedDebugInfo && g_checkLeaks && (type_info->flags_ & TF_LEAK_DETECTOR_CANDIDATE) != 0) { - // Add newly allocated object to the double-linked list of potentially cyclic objects. - MetaObjHeader* meta = obj->meta_object(); - lock(&g_leakCheckerGlobalLock); - KRef old = g_leakCheckerGlobalList; - g_leakCheckerGlobalList = obj; - meta->LeakDetector.next_ = old; - if (old != nullptr) - old->meta_object()->LeakDetector.previous_ = obj; - unlock(&g_leakCheckerGlobalLock); - } #if USE_CYCLIC_GC if ((obj->type_info()->flags_ & TF_LEAK_DETECTOR_CANDIDATE) != 0) { cyclicAddAtomicRoot(obj); @@ -2629,100 +2595,6 @@ void shareAny(ObjHeader* obj) { container->makeShared(); } -OBJ_GETTER0(detectCyclicReferences) { - // Collect rootset, hold references to simplify remaining code. - KRefList rootset; - lock(&g_leakCheckerGlobalLock); - auto* candidate = g_leakCheckerGlobalList; - while (candidate != nullptr) { - addHeapRef(candidate); - rootset.push_back(candidate); - candidate = candidate->meta_object()->LeakDetector.next_; - } - unlock(&g_leakCheckerGlobalLock); - KRefSet cyclic; - KRefSet seen; - KRefDeque toVisit; - for (auto* root: rootset) { - seen.clear(); - toVisit.clear(); - traverseReferredObjects(root, [&toVisit](ObjHeader* obj) { toVisit.push_front(obj); }); - bool seenToRoot = false; - while (!toVisit.empty() && !seenToRoot) { - KRef current = toVisit.front(); - toVisit.pop_front(); - if (cyclic.count(current) != 0) continue; - if (current == root) seenToRoot = true; - // TODO: racy against concurrent mutators. - if (seen.count(current) == 0) { - traverseReferredObjects(current, [&toVisit](ObjHeader* obj) { - toVisit.push_front(obj); - }); - seen.insert(current); - } - } - if (seenToRoot) { - cyclic.insert(root); - } - } - int numElements = cyclic.size(); - ArrayHeader* result = AllocArrayInstance(theArrayTypeInfo, numElements, OBJ_RESULT)->array(); - KRef* place = ArrayAddressOfElementAt(result, 0); - for (auto* it: cyclic) { - UpdateHeapRef(place++, it); - } - for (auto* root: rootset) { - ReleaseHeapRef(root); - } - RETURN_OBJ(result->obj()); -} - -OBJ_GETTER(findCycle, KRef root) { - KRefSet seen; - KRefListDeque queue; - KRefDeque toVisit; - KRefList path; - traverseReferredObjects(root, [&toVisit](ObjHeader* obj) { toVisit.push_front(obj); }); - bool isFound = false; - while (!toVisit.empty() && !isFound) { - KRef current = toVisit.front(); - toVisit.pop_front(); - // Do DFS path search. - KRefList first; - first.push_back(current); - queue.emplace_back(first); - seen.clear(); - while (!queue.empty()) { - KRefList currentPath = queue.back(); - queue.pop_back(); - KRef node = currentPath[currentPath.size() - 1]; - if (node == root) { - isFound = true; - path = currentPath; - break; - } - if (seen.count(node) == 0) { - // TODO: racy against concurrent mutators. - traverseReferredObjects(node, [&queue, ¤tPath](ObjHeader* obj) { - KRefList newPath(currentPath); - newPath.push_back(obj); - queue.emplace_back(newPath); - }); - seen.insert(node); - } - } - } - ArrayHeader* result = nullptr; - if (isFound) { - result = AllocArrayInstance(theArrayTypeInfo, path.size(), OBJ_RESULT)->array(); - KRef* place = ArrayAddressOfElementAt(result, 0); - for (auto* it: path) { - UpdateHeapRef(place++, it); - } - } - RETURN_OBJ(result->obj()); -} - } // namespace MetaObjHeader* ObjHeader::createMetaObject(TypeInfo** location) { @@ -3132,15 +3004,6 @@ KBoolean Kotlin_native_internal_GC_getTuneThreshold(KRef) { #endif } -OBJ_GETTER(Kotlin_native_internal_GC_detectCycles, KRef) { - if (!KonanNeedDebugInfo || !g_checkLeaks) RETURN_OBJ(nullptr); - RETURN_RESULT_OF0(detectCyclicReferences); -} - -OBJ_GETTER(Kotlin_native_internal_GC_findCycle, KRef, KRef root) { - RETURN_RESULT_OF(findCycle, root); -} - KNativePtr CreateStablePointer(KRef any) { return createStablePointer(any); } diff --git a/runtime/src/main/cpp/Memory.h b/runtime/src/main/cpp/Memory.h index 64c41c00321..8fdb5803d7b 100644 --- a/runtime/src/main/cpp/Memory.h +++ b/runtime/src/main/cpp/Memory.h @@ -325,17 +325,10 @@ struct MetaObjHeader { // Flags for the object state. int32_t flags_; - // TODO: maybe make it a union for the orthogonal features. struct { // Strong reference to the counter object. ObjHeader* counter_; } WeakReference; - struct { - // Leak detector's previous list element. - ObjHeader* previous_; - // Leak detector's next list element. - ObjHeader* next_; - } LeakDetector; }; // Header of every object. diff --git a/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt b/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt index a5f370657e9..e9d1232aa38 100644 --- a/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt +++ b/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt @@ -216,8 +216,7 @@ private fun debugString(value: Any?): String { /** * An atomic reference to a frozen Kotlin object. Can be used in concurrent scenarious * but frequently shall be of nullable type and be zeroed out once no longer needed. - * Otherwise memory leak could happen. To detect such leaks [kotlin.native.internal.GC.detectCycles] - * in debug mode could be helpful. + * Asynchronous cycle collector takes care of cyclic references of that kind. */ @Frozen @LeakDetectorCandidate @@ -293,9 +292,8 @@ public class AtomicReference { /** * An atomic reference to a Kotlin object. Can be used in concurrent scenarious, but must be frozen first, - * otherwise behaves as regular box for the value. If frozen, shall be zeroed out once no longer needed. - * Otherwise memory leak could happen. To detect such leaks [kotlin.native.internal.GC.detectCycles] - * in debug mode could be helpful. + * otherwise behaves as regular box for the value. Asynchronous cycle collector helps to collect the + * cyclic garbage going through frozen instances of `FreezableAtomicReference`. */ @NoReorderFields @LeakDetectorCandidate diff --git a/runtime/src/main/kotlin/kotlin/native/internal/GC.kt b/runtime/src/main/kotlin/kotlin/native/internal/GC.kt index 5aeef5c5ca8..5f0d0906312 100644 --- a/runtime/src/main/kotlin/kotlin/native/internal/GC.kt +++ b/runtime/src/main/kotlin/kotlin/native/internal/GC.kt @@ -92,24 +92,6 @@ object GC { get() = getCyclicCollectorEnabled() set(value) = setCyclicCollectorEnabled(value) - /** - * Detect cyclic references going via atomic references and return list of cycle-inducing objects - * or `null` if the leak detector is not available. Use [Platform.isMemoryLeakCheckerActive] to check - * leak detector availability. - * Note that cycle detector requires reference graph stability, thus it may not work as - * expected or even crash for mutating graphs. - */ - @SymbolName("Kotlin_native_internal_GC_detectCycles") - external fun detectCycles(): Array? - - /** - * Find a reference cycle including from the given object, `null` if no cycles detected. - * Note that cycle detector requires reference graph stability, thus it may not work as - * expected or even crash for mutating graphs. - */ - @SymbolName("Kotlin_native_internal_GC_findCycle") - external fun findCycle(root: Any): Array? - @SymbolName("Kotlin_native_internal_GC_getThreshold") private external fun getThreshold(): Int