diff --git a/kotlin-native/backend.native/tests/objcexport/expectedLazy.h b/kotlin-native/backend.native/tests/objcexport/expectedLazy.h index 146d6fa10d0..c0d5af93f2c 100644 --- a/kotlin-native/backend.native/tests/objcexport/expectedLazy.h +++ b/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)); diff --git a/kotlin-native/backend.native/tests/objcexport/expectedLazyLegacySuspendUnit.h b/kotlin-native/backend.native/tests/objcexport/expectedLazyLegacySuspendUnit.h index e1f27b7217d..dc754402b25 100644 --- a/kotlin-native/backend.native/tests/objcexport/expectedLazyLegacySuspendUnit.h +++ b/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)); diff --git a/kotlin-native/backend.native/tests/objcexport/expectedLazyNoGenerics.h b/kotlin-native/backend.native/tests/objcexport/expectedLazyNoGenerics.h index 84982274eca..fd8308ef6d5 100644 --- a/kotlin-native/backend.native/tests/objcexport/expectedLazyNoGenerics.h +++ b/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)); diff --git a/kotlin-native/backend.native/tests/objcexport/values.kt b/kotlin-native/backend.native/tests/objcexport/values.kt index cdf2ab729f8..70bfc30e9a4 100644 --- a/kotlin-native/backend.native/tests/objcexport/values.kt +++ b/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 diff --git a/kotlin-native/backend.native/tests/objcexport/values.swift b/kotlin-native/backend.native/tests/objcexport/values.swift index 03f4b80dd65..660def5d890 100644 --- a/kotlin-native/backend.native/tests/objcexport/values.swift +++ b/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) } } diff --git a/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp b/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp index a49cefedd3a..d45d4f79f90 100644 --- a/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp +++ b/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--; }