Native, old MM: fix releasing foreign refs to circular frozen graphs

^KT-49497

kotlin-native/backend.native/tests/objcexport/expectedLazy.h

@@ -2433,6 +2433,13 @@ __attribute__((swift_name("TestRememberNewObject")))
 - (void)waitForCleanup __attribute__((swift_name("waitForCleanup()")));
 @end;
 
+__attribute__((objc_subclassing_restricted))
+__attribute__((swift_name("KT49497Model")))
+@interface KtKT49497Model : KtBase
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
++ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end;
+
 __attribute__((swift_name("ClassForTypeCheck")))
 @interface KtClassForTypeCheck : KtBase
 - (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));

kotlin-native/backend.native/tests/objcexport/expectedLazyLegacySuspendUnit.h

@@ -2375,6 +2375,13 @@ __attribute__((swift_name("TestRememberNewObject")))
 - (void)waitForCleanup __attribute__((swift_name("waitForCleanup()")));
 @end;
 
+__attribute__((objc_subclassing_restricted))
+__attribute__((swift_name("KT49497Model")))
+@interface KtKT49497Model : KtBase
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
++ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end;
+
 __attribute__((swift_name("ClassForTypeCheck")))
 @interface KtClassForTypeCheck : KtBase
 - (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));

kotlin-native/backend.native/tests/objcexport/expectedLazyNoGenerics.h

@@ -2375,6 +2375,13 @@ __attribute__((swift_name("TestRememberNewObject")))
 - (void)waitForCleanup __attribute__((swift_name("waitForCleanup()")));
 @end;
 
+__attribute__((objc_subclassing_restricted))
+__attribute__((swift_name("KT49497Model")))
+@interface KtKT49497Model : KtBase
+- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));
++ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead")));
+@end;
+
 __attribute__((swift_name("ClassForTypeCheck")))
 @interface KtClassForTypeCheck : KtBase
 - (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer));

kotlin-native/backend.native/tests/objcexport/values.kt

@@ -896,6 +896,17 @@ fun testRememberNewObject(test: TestRememberNewObject) {
     assertNotEquals("", obj.toString()) // Likely crashes if object is removed.
 }
 
+class KT49497Model {
+    private class SelfRef(val self: KT49497Model)
+
+    // Wrapping `this` to make the strongly connected component non-trival, just in case:
+    private val selfRef = SelfRef(this)
+
+    init {
+        freeze()
+    }
+}
+
 open class ClassForTypeCheck
 
 fun testClassTypeCheck(x: Any) = x is ClassForTypeCheck

kotlin-native/backend.native/tests/objcexport/values.swift

@@ -1123,6 +1123,24 @@ class TestSharedRefs {
         try assertFalse(refs.hasAliveObjects())
     }
 
+    // Based on https://youtrack.jetbrains.com/issue/KT-49497.
+
+    func testKT49497() throws {
+        var model: KT49497Model? = nil
+
+        for i in 1...10 {
+            model = KT49497Model() // Frozen and has a reference to itself, so becomes aggregating frozen container.
+            ValuesKt.gc() // Just in case, to ensure there are no other references except `model`.
+
+            runInNewThread(initializeKotlinRuntime: false) {
+                // Thread has no runtime initialized, so this should enqueue release ref to the original thread:
+                model = nil
+            }
+
+            ValuesKt.gc() // Process the enqueued release ref.
+        }
+    }
+
     func test() throws {
         try testLambdaSimple()
         try testObjectPartialRelease()
@@ -1144,6 +1162,8 @@ class TestSharedRefs {
         try testRememberNewObject(createObject: { $0.createFrozenCollection() })
 #endif
 
+        try testKT49497()
+
         usleep(300 * 1000)
     }
 }

kotlin-native/runtime/src/legacymm/cpp/Memory.cpp

@@ -1806,6 +1806,28 @@ void processDecrements(MemoryState* state) {
   RuntimeAssert(IsStrictMemoryModel(), "Only works in strict model now");
   auto* toRelease = state->toRelease;
   state->gcSuspendCount++;
+
+  state->foreignRefManager->processEnqueuedReleaseRefsWith([](ObjHeader* obj) {
+    ContainerHeader* container = containerFor(obj);
+    if (container != nullptr) decrementRC(container);
+  });
+
+  // The code above can call freeContainer if RC drops to zero.
+  // freeContainer can in turn call ZeroHeapRef for freed object fields, which does enqueueDecrementRC.
+  // The latter are processed by the code below. So if we reorder this, the enqueued decrement RC operations
+  // will have to wait for the next GC unnecessarily.
+  //
+  // In addition to being inefficient, the reordering causes https://youtrack.jetbrains.com/issue/KT-49497:
+  // The foreign ref to the frozen aggregating container gets released and causes freeContainer,
+  // which in turn enqueues decrement RC for the same aggregating container.
+  // The enqueued operations outlive processFinalizerQueue at the end of this GC, so during the next GC
+  // they hit the reclaimed memory.
+  // This is generally a special case of the known issue with frozen aggregating containers,
+  // see "TODO: enable me, once account for inner references in frozen objects correctly." above.
+  //
+  // With the current order in this code, wrong accounting for inner references also happens,
+  // but it doesn't do more harm than with regular (non-foreign) references (supposedly only breaks invariants locally).
+
   while (toRelease->size() > 0) {
      auto* container = toRelease->back();
      toRelease->pop_back();
@@ -1816,10 +1838,6 @@ void processDecrements(MemoryState* state) {
      decrementRC(container);
   }
 
-  state->foreignRefManager->processEnqueuedReleaseRefsWith([](ObjHeader* obj) {
-    ContainerHeader* container = containerFor(obj);
-    if (container != nullptr) decrementRC(container);
-  });
   state->gcSuspendCount--;
 }