[K/N] Immediately destroy objects that finalize only via ExtraObjectData ^KT-63423

kotlin-native/runtime/src/alloc/common/cpp/AllocatorTestSupport.hpp

@@ -15,5 +15,6 @@ namespace kotlin::alloc::test_support {
 
 void assertClear(Allocator& allocator) noexcept;
 std::vector<ObjHeader*> allocatedObjects(mm::ThreadData& threadData) noexcept;
+void detachAndDestroyExtraObjectData(mm::ExtraObjectData& extraObject) noexcept;
 
 } // namespace kotlin::alloc::test_support

kotlin-native/runtime/src/alloc/custom/cpp/AllocatorImplTestSupport.cpp

@@ -33,3 +33,10 @@ void alloc::test_support::assertClear(Allocator& allocator) noexcept {
 std::vector<ObjHeader*> alloc::test_support::allocatedObjects(mm::ThreadData& threadData) noexcept {
     return threadData.allocator().impl().alloc().heap().GetAllocatedObjects();
 }
+
+void alloc::test_support::detachAndDestroyExtraObjectData(mm::ExtraObjectData& extraObject) noexcept {
+    extraObject.ClearRegularWeakReferenceImpl();
+    extraObject.UnlinkFromBaseObject();
+    destroyExtraObjectData(extraObject);
+    extraObject.setFlag(mm::ExtraObjectData::FLAGS_SWEEPABLE);
+}

kotlin-native/runtime/src/alloc/custom/cpp/CustomFinalizerProcessor.hpp

@@ -23,8 +23,13 @@ struct FinalizerQueueTraits {
     static void process(FinalizerQueue queue) noexcept {
         while (auto* cell = queue.Pop()) {
             auto* extraObject = cell->Data();
-            auto* baseObject = extraObject->GetBaseObject();
+            if (auto* baseObject = extraObject->GetBaseObject()) {
-            RunFinalizers(baseObject);
+                RunFinalizers(baseObject);
+            } else {
+                // This `ExtraObjectData` does not have an object attached. This means
+                // that the only finalization step is destroying it.
+                destroyExtraObjectData(*extraObject);
+            }
         }
     }
 };

kotlin-native/runtime/src/alloc/custom/cpp/GCApi.cpp

@@ -20,6 +20,7 @@
 #include "CustomLogging.hpp"
 #include "ExtraObjectData.hpp"
 #include "ExtraObjectPage.hpp"
+#include "FinalizerHooks.hpp"
 #include "GC.hpp"
 #include "GCStatistics.hpp"
 #include "KAssert.h"
@@ -49,9 +50,17 @@ bool SweepObject(uint8_t* object, FinalizerQueue& finalizerQueue, gc::GCHandle::
             extraObject->ClearRegularWeakReferenceImpl();
             CustomAllocDebug("SweepObject: fromExtraObject(%p) = %p", extraObject, ExtraObjectCell::fromExtraObject(extraObject));
             finalizerQueue.Push(ExtraObjectCell::fromExtraObject(extraObject));
-            gcHandle.addMarkedObject();
+            if (HasFinalizersDataInObject(heapObjHeader->object())) {
-            gcHandle.addKeptObject(size);
+                // The object must survive until the finalizers for it are finished.
-            return true;
+                gcHandle.addMarkedObject();
+                gcHandle.addKeptObject(size);
+                return true;
+            }
+            // The object has a finalizer, but all the data for it resides in `ExtraObjectData`. So, detach the object from it, and free it.
+            extraObject->UnlinkFromBaseObject();
+            CustomAllocDebug("SweepObject(%p): can be reclaimed", heapObjHeader);
+            gcHandle.addSweptObject();
+            return false;
         }
         if (!extraObject->getFlag(mm::ExtraObjectData::FLAGS_FINALIZED)) {
             CustomAllocDebug("SweepObject(%p): already waiting to be finalized", heapObjHeader);

kotlin-native/runtime/src/alloc/legacy/cpp/AllocatorImplTestSupport.cpp

@@ -51,3 +51,8 @@ std::vector<ObjHeader*> alloc::test_support::allocatedObjects(mm::ThreadData& th
     }
     return objects;
 }
+
+void alloc::test_support::detachAndDestroyExtraObjectData(mm::ExtraObjectData& extraObject) noexcept {
+    extraObject.ClearRegularWeakReferenceImpl();
+    destroyExtraObjectData(extraObject);
+}

kotlin-native/runtime/src/alloc/legacy/cpp/ExtraObjectDataFactory.hpp

@@ -30,6 +30,14 @@ public:
         // when it's asked by GC to stop.
         using Producer::Publish;
 
+        void ClearForTests() noexcept {
+            forEachNode([](auto& extraObject) noexcept {
+                extraObject.ClearRegularWeakReferenceImpl();
+                extraObject.Uninstall();
+            });
+            Producer::ClearForTests();
+        }
+
         // Do not add fields as this is just a wrapper and Producer does not have virtual destructor.
     };
 
@@ -43,7 +51,13 @@ public:
     // Lock registry for safe iteration.
     Iterable LockForIter() noexcept { return extraObjects_.LockForIter(); }
 
-    void ClearForTests() noexcept { extraObjects_.ClearForTests(); }
+    void ClearForTests() noexcept {
+        for (auto& extraObject : extraObjects_.LockForIter()) {
+            extraObject.ClearRegularWeakReferenceImpl();
+            extraObject.Uninstall();
+        }
+        extraObjects_.ClearForTests();
+    }
 
     size_t GetSizeUnsafe() noexcept { return extraObjects_.GetSizeUnsafe(); }
 

kotlin-native/runtime/src/gc/common/cpp/TracingGCTest.hpp

@@ -1234,17 +1234,6 @@ TYPED_TEST_P(TracingGCTest, WeakResuractionInMark) {
     for (auto& future : mutatorFutures) {
         future.wait();
     }
-
-    for (int i = 0; i < kDefaultThreadCount; ++i) {
-        mutators[i].Execute([&, i](mm::ThreadData&, Mutator&) noexcept {
-            if (auto weakReferee = weaks[i]->get()) {
-                auto& extraObj = *mm::ExtraObjectData::Get(weakReferee);
-                extraObj.ClearRegularWeakReferenceImpl();
-                extraObj.Uninstall();
-                alloc::destroyExtraObjectData(extraObj);
-            }
-        }).wait();
-    }
 }
 
 #define TRACING_GC_TEST_LIST \
@@ -1356,11 +1345,6 @@ TYPED_TEST_P(STWMarkGCTest, MultipleMutatorsWeakNewObj) {
             })();
             EXPECT_NE(objectWeak.get(), nullptr);
 
-            auto& extraObj = *mm::ExtraObjectData::Get(object.header());
-            extraObj.ClearRegularWeakReferenceImpl();
-            extraObj.Uninstall();
-            alloc::destroyExtraObjectData(extraObj);
-
             while (!gcDone.load(std::memory_order_relaxed)) {
                 mm::safePoint(threadData);
             }

kotlin-native/runtime/src/main/cpp/FinalizerHooks.cpp

@@ -35,7 +35,11 @@ NO_INLINE void RunFinalizerHooksImpl(ObjHeader* object, const TypeInfo* type) no
 } // namespace
 
 ALWAYS_INLINE bool kotlin::HasFinalizers(ObjHeader* object) noexcept {
-    return object->has_meta_object() || (object->type_info()->flags_ & TF_HAS_FINALIZER) != 0;
+    return object->has_meta_object() || HasFinalizersDataInObject(object);
+}
+
+ALWAYS_INLINE bool kotlin::HasFinalizersDataInObject(ObjHeader* object) noexcept {
+    return (object->type_info()->flags_ & TF_HAS_FINALIZER) != 0;
 }
 
 ALWAYS_INLINE void kotlin::RunFinalizers(ObjHeader* object) noexcept {

kotlin-native/runtime/src/main/cpp/FinalizerHooks.hpp

@@ -14,6 +14,7 @@ namespace kotlin {
 // finalizer must never try to reference them.
 
 bool HasFinalizers(ObjHeader* object) noexcept;
+bool HasFinalizersDataInObject(ObjHeader* object) noexcept;
 void RunFinalizers(ObjHeader* object) noexcept;
 
 void SetFinalizerHookForTesting(void (*hook)(ObjHeader*)) noexcept;

kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp

@@ -102,6 +102,12 @@ public:
             deletionQueue_.clear();
         }
 
+    protected:
+        template <typename F>
+        void forEachNode(F&& f) noexcept(noexcept(f(std::declval<T&>()))) {
+            for (auto& node : queue_) f(*node);
+        }
+
     private:
         MultiSourceQueue& owner_; // weak
         List<Node> queue_;

kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp

@@ -40,7 +40,10 @@ mm::ExtraObjectData& mm::ExtraObjectData::Install(ObjHeader* object) noexcept {
 }
 
 void mm::ExtraObjectData::UnlinkFromBaseObject() noexcept {
-    auto *object = GetBaseObject();
+    auto* object = weakReferenceOrBaseObject_.exchange(nullptr);
+    RuntimeAssert(
+            !hasPointerBits(object, WEAK_REF_TAG), "ExtraObjectData %p has uncleared weak reference %p during unlink", this,
+            clearPointerBits(object, WEAK_REF_TAG));
     atomicSetRelease(const_cast<const TypeInfo**>(&object->typeInfoOrMeta_), typeInfo_);
     RuntimeAssert(
             !object->has_meta_object(), "Object %p has metaobject %p after removing metaobject %p", object, object->meta_object_or_null(),

kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp

@@ -10,6 +10,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include "AllocatorTestSupport.hpp"
 #include "ObjectTestSupport.hpp"
 #include "ScopedThread.hpp"
 #include "TestSupport.hpp"
@@ -51,8 +52,7 @@ TEST_F(ExtraObjectDataTest, Install) {
     EXPECT_FALSE(extraData.HasRegularWeakReferenceImpl());
     EXPECT_THAT(extraData.GetBaseObject(), object.header());
 
-    extraData.Uninstall();
+    alloc::test_support::detachAndDestroyExtraObjectData(extraData);
-    mm::GlobalData::Instance().threadRegistry().CurrentThreadData()->ClearForTests();
 
     EXPECT_FALSE(object.header()->has_meta_object());
     EXPECT_THAT(object.header()->type_info(), typeInfo);