Fix deadlock in cyclic collector

Avoid reentering the lock in
releasePendingUnlocked -> ZeroHeapRef -> garbageCollect

backend.native/tests/build.gradle

@@ -2787,6 +2787,10 @@ standaloneTest("cycle_collector") {
     source = "runtime/memory/cycle_collector.kt"
 }
 
+standaloneTest("cycle_collector_deadlock1") {
+    source = "runtime/memory/cycle_collector_deadlock1.kt"
+}
+
 standaloneTest("leakMemory") {
     disabled = project.globalTestArgs.contains('-opt') || (project.testTarget == 'wasm32') // Needs debug build.
     source = "runtime/memory/leak_memory.kt"

backend.native/tests/runtime/memory/cycle_collector_deadlock1.kt

@@ -0,0 +1,16 @@
+import kotlin.native.concurrent.*
+import kotlin.native.internal.GC
+import kotlin.test.*
+
+fun main() {
+    kotlin.native.internal.GC.cyclicCollectorEnabled = true
+
+    repeat(10000) {
+        // Create atomic cyclic garbage:
+        val ref = AtomicReference<Any?>(null)
+        ref.value = ref
+    }
+
+    // main thread will then run cycle collector termination, which involves running it and cleaning everything up.
+    // 10000 references should hit [kGcThreshold] then.
+}

runtime/src/main/cpp/CyclicCollector.cpp

@@ -21,6 +21,7 @@
 #include "Atomic.h"
 #include "KAssert.h"
 #include "Memory.h"
+#include "MemoryPrivate.hpp"
 #include "Natives.h"
 #include "Porting.h"
 #include "Types.h"
@@ -381,17 +382,31 @@ class CyclicCollector {
     // We are not doing that on the UI thread, as taking lock is slow, unless
     // it happens on deinit of the collector or if there are no other workers.
     if ((atomicGet(&pendingRelease_) != 0) && ((worker != mainWorker_) || (currentAliveWorkers_ == 1))) {
-      suggestLockRelease();
+      KStdVector<ObjHeader*> heapRefsToRelease;
-      Locker locker(&lock_);
+
-      COLLECTOR_LOG("clearing %d release candidates on %p\n", toRelease_.size(), worker);
+      {
-      for (auto* it: toRelease_) {
+        suggestLockRelease();
-        COLLECTOR_LOG("clear references in %p\n", it)
+        Locker locker(&lock_);
-        traverseObjectFields(it, [](ObjHeader** location) {
+        COLLECTOR_LOG("clearing %d release candidates on %p\n", toRelease_.size(), worker);
-          ZeroHeapRef(location);
+        for (auto* it: toRelease_) {
-        });
+          COLLECTOR_LOG("clear references in %p\n", it)
+          traverseObjectFields(it, [&heapRefsToRelease](ObjHeader** location) {
+            // Avoid using ZeroHeapRef here: it can provoke garbageCollect() which would then stuck on taking [lock_]
+            // (which is already taken above).
+            auto* value = *location;
+            if (reinterpret_cast<uintptr_t>(value) > 1) {
+              *location = nullptr;
+              heapRefsToRelease.push_back(value);
+            }
+          });
+        }
+        toRelease_.clear();
+        atomicSet(&pendingRelease_, 0);
+      }
+
+      for (auto* it: heapRefsToRelease) {
+        ReleaseHeapRef(it);
       }
-      toRelease_.clear();
-      atomicSet(&pendingRelease_, 0);
     }
   }