diff --git a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp index 067a87b3c25..803df164b16 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp +++ b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp @@ -71,9 +71,16 @@ void SweepExtraObjects(typename Traits::ExtraObjectsFactory& objectFactory) noex auto &extraObject = *it; if (!Traits::IsMarkedByExtraObject(extraObject)) { extraObject.ClearWeakReferenceCounter(); - extraObject.DetachAssociatedObject(); + if (extraObject.HasAssociatedObject()) { + extraObject.DetachAssociatedObject(); + ++it; + } else { + extraObject.Uninstall(); + objectFactory.EraseAndAdvance(it); + } + } else { + ++it; } - ++it; } } diff --git a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsMarkTest.cpp b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsMarkTest.cpp index a7178d2eda9..82f28f98bf5 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsMarkTest.cpp +++ b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsMarkTest.cpp @@ -50,6 +50,7 @@ public: auto& extraObjectData = mm::ExtraObjectData::GetOrInstall(GetObjHeader()); auto *setCounter = extraObjectData.GetOrSetWeakReferenceCounter(GetObjHeader(), counter.GetObjHeader()); EXPECT_EQ(setCounter, counter.GetObjHeader()); + EXPECT_EQ(extraObjectData.GetBaseObject(), GetObjHeader()); } protected: @@ -340,6 +341,7 @@ TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleCharArrayWithExtraData) { TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleObjectWithWeakCounter) { Object weakCounter; Object object; + weakCounter->field1 = object.header(); object.InstallWeakCounter(weakCounter); auto stats = Mark({object}); @@ -352,6 +354,7 @@ TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleObjectWithWeakCounter) { TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleObjectArrayWithWeakCounter) { Object weakCounter; ObjectArray array; + weakCounter->field1 = array.header(); array.InstallWeakCounter(weakCounter); auto stats = Mark({array}); @@ -364,6 +367,7 @@ TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleObjectArrayWithWeakCounter) { TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleCharArrayWithWeakCounter) { Object weakCounter; CharArray array; + weakCounter->field1 = array.header(); array.InstallWeakCounter(weakCounter); auto stats = Mark({array}); @@ -377,6 +381,7 @@ TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleObjectWithInvalidFieldsWithWeakCount Object weakCounter; Object object; object->field1 = kInitializingSingleton; + weakCounter->field1 = object.header(); object.InstallWeakCounter(weakCounter); auto stats = Mark({object}); @@ -390,6 +395,7 @@ TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleObjectArrayWithInvalidFieldsWithWeak Object weakCounter; ObjectArray array; array.elements()[0] = kInitializingSingleton; + weakCounter->field1 = array.header(); array.InstallWeakCounter(weakCounter); auto stats = Mark({array}); @@ -405,6 +411,7 @@ TEST_F(MarkAndSweepUtilsMarkTest, MarkSingleCharArrayWithSomeDataWithWeakCounter array.elements()[0] = 'a'; array.elements()[1] = 'b'; array.elements()[2] = 'c'; + weakCounter->field1 = array.header(); array.InstallWeakCounter(weakCounter); auto stats = Mark({array}); diff --git a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp index 83fb550b107..2361ab10566 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp +++ b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp @@ -177,8 +177,7 @@ public: ~MarkAndSweepUtilsSweepTest() override { auto deallocExtraObject = [this](ObjHeader* obj) { auto *extraObject = mm::ExtraObjectData::Get(obj); - EXPECT_NE(extraObject, nullptr); - obj->typeInfoOrMeta_ = const_cast(obj->type_info()); + extraObject->Uninstall(); extraObjectFactory_.DestroyExtraObjectData(extraObjectFactoryThreadQueue_, *extraObject); extraObjectFactoryThreadQueue_.Publish(); }; @@ -223,6 +222,14 @@ public: return objects; } + KStdVector AliveExtraObjects() { + KStdVector objects; + for (auto &node : extraObjectFactory_.LockForIter()) { + objects.push_back(&node); + } + return objects; + } + Object& AllocateObject(const TypeInfo* typeInfo = typeHolder.typeInfo()) { auto* object = objectFactoryThreadQueue_.CreateObject(typeInfo); objectFactoryThreadQueue_.Publish(); @@ -357,9 +364,9 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleObjectWithExtraData) { auto finalizers = Sweep(); - EXPECT_THAT(finalizers, testing::UnorderedElementsAre(object.header())); + EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre()); - EXPECT_THAT(object.state(), GC::ObjectData::State::kUnmarked); + EXPECT_THAT(AliveExtraObjects(), testing::UnorderedElementsAre()); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleObjectArrayWithExtraData) { @@ -369,9 +376,9 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleObjectArrayWithExtraData) { auto finalizers = Sweep(); - EXPECT_THAT(finalizers, testing::UnorderedElementsAre(array.header())); + EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre()); - EXPECT_THAT(array.state(), GC::ObjectData::State::kUnmarked); + EXPECT_THAT(AliveExtraObjects(), testing::UnorderedElementsAre()); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleCharArrayWithExtraData) { @@ -381,14 +388,14 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleCharArrayWithExtraData) { auto finalizers = Sweep(); - EXPECT_THAT(finalizers, testing::UnorderedElementsAre(array.header())); + EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre()); - EXPECT_THAT(array.state(), GC::ObjectData::State::kUnmarked); + EXPECT_THAT(AliveExtraObjects(), testing::UnorderedElementsAre()); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleMarkedObjectWithExtraData) { auto& object = AllocateObject(); - InstallExtraData(object.header()); + auto& extra = InstallExtraData(object.header()); object.Mark(); ASSERT_THAT(Alive(), testing::UnorderedElementsAre(object.header())); @@ -396,12 +403,12 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleMarkedObjectWithExtraData) { EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre(object.header())); - EXPECT_THAT(object.state(), GC::ObjectData::State::kMarkReset); + EXPECT_THAT(AliveExtraObjects(), testing::UnorderedElementsAre(&extra)); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleMarkedObjectArrayWithExtraData) { auto& array = AllocateObjectArray(); - InstallExtraData(array.header()); + auto& extra = InstallExtraData(array.header()); array.Mark(); ASSERT_THAT(Alive(), testing::UnorderedElementsAre(array.header())); @@ -409,12 +416,12 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleMarkedObjectArrayWithExtraData) { EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre(array.header())); - EXPECT_THAT(array.state(), GC::ObjectData::State::kMarkReset); + EXPECT_THAT(AliveExtraObjects(), testing::UnorderedElementsAre(&extra)); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleMarkedCharArrayWithExtraData) { auto& array = AllocateCharArray(); - InstallExtraData(array.header()); + auto& extra = InstallExtraData(array.header()); array.Mark(); ASSERT_THAT(Alive(), testing::UnorderedElementsAre(array.header())); @@ -422,7 +429,7 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleMarkedCharArrayWithExtraData) { EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre(array.header())); - EXPECT_THAT(array.state(), GC::ObjectData::State::kMarkReset); + EXPECT_THAT(AliveExtraObjects(), testing::UnorderedElementsAre(&extra)); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleObjectWithFinalizerHook) { @@ -457,10 +464,8 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleObjectWithWeakCounter) { auto finalizers = Sweep(); - EXPECT_THAT(finalizers, testing::UnorderedElementsAre(object.header())); + EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre()); - EXPECT_THAT(object.state(), GC::ObjectData::State::kUnmarked); - EXPECT_FALSE(object.HasWeakCounter()); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleObjectArrayWithWeakCounter) { @@ -470,10 +475,8 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleObjectArrayWithWeakCounter) { auto finalizers = Sweep(); - EXPECT_THAT(finalizers, testing::UnorderedElementsAre(array.header())); + EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre()); - EXPECT_THAT(array.state(), GC::ObjectData::State::kUnmarked); - EXPECT_FALSE(array.HasWeakCounter()); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleCharArrayWithWeakCounter) { @@ -483,10 +486,8 @@ TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleCharArrayWithWeakCounter) { auto finalizers = Sweep(); - EXPECT_THAT(finalizers, testing::UnorderedElementsAre(array.header())); + EXPECT_THAT(finalizers, testing::UnorderedElementsAre()); EXPECT_THAT(Alive(), testing::UnorderedElementsAre()); - EXPECT_THAT(array.state(), GC::ObjectData::State::kUnmarked); - EXPECT_FALSE(array.HasWeakCounter()); } TEST_F(MarkAndSweepUtilsSweepTest, SweepSingleMarkedObjectWithWeakCounter) { diff --git a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp index 48ccae4575b..6a4171a27ed 100644 --- a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp +++ b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp @@ -160,6 +160,11 @@ public: deletionQueue_ = std::move(remainingDeletions); } + // requires LockForIter + void EraseAndAdvance(Iterator &it) { + it.position_ = queue_.erase(it.position_); + } + void ClearForTests() noexcept { queue_.clear(); deletionQueue_.clear(); diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp index fca513e3b70..7d7a7d1350a 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp @@ -56,20 +56,15 @@ mm::ExtraObjectData& mm::ExtraObjectData::Install(ObjHeader* object) noexcept { return data; } -// static -void mm::ExtraObjectData::Uninstall(ObjHeader* object) noexcept { - RuntimeAssert(object->has_meta_object(), "Object must have a meta object set"); +void mm::ExtraObjectData::Uninstall() noexcept { + auto *object = GetBaseObject(); + *const_cast(&object->typeInfoOrMeta_) = typeInfo_; + RuntimeAssert(!object->has_meta_object(), "Object has metaobject after removing metaobject"); - auto& data = ExtraObjectData::FromMetaObjHeader(object->meta_object()); - - *const_cast(&object->typeInfoOrMeta_) = data.typeInfo_; - - auto *threadData = mm::ThreadRegistry::Instance().CurrentThreadData(); #ifdef KONAN_OBJC_INTEROP - Kotlin_ObjCExport_releaseAssociatedObject(data.associatedObject_); - data.associatedObject_ = nullptr; + Kotlin_ObjCExport_releaseAssociatedObject(associatedObject_); + associatedObject_ = nullptr; #endif - mm::ExtraObjectDataFactory::Instance().DestroyExtraObjectData(threadData, data); } void mm::ExtraObjectData::DetachAssociatedObject() noexcept { @@ -78,6 +73,15 @@ void mm::ExtraObjectData::DetachAssociatedObject() noexcept { #endif } +bool mm::ExtraObjectData::HasAssociatedObject() noexcept { +#ifdef KONAN_OBJC_INTEROP + return associatedObject_ != nullptr; +#else + return false; +#endif +} + + void mm::ExtraObjectData::ClearWeakReferenceCounter() noexcept { if (!HasWeakReferenceCounter()) return; diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp index f1adb88dfe1..982891b51d2 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp @@ -41,11 +41,12 @@ public: static ExtraObjectData& GetOrInstall(ObjHeader* object) noexcept { return FromMetaObjHeader(object->meta_object()); } static ExtraObjectData& Install(ObjHeader* object) noexcept; - static void Uninstall(ObjHeader* object) noexcept; + void Uninstall() noexcept; #ifdef KONAN_OBJC_INTEROP void** GetAssociatedObjectLocation() noexcept { return &associatedObject_; } #endif + bool HasAssociatedObject() noexcept; void DetachAssociatedObject() noexcept; std::atomic& flags() noexcept { return flags_; } diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataFactory.hpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataFactory.hpp index a04c29619d1..26b68dee67b 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataFactory.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataFactory.hpp @@ -53,6 +53,9 @@ public: size_t GetSizeUnsafe() noexcept { return extraObjects_.GetSizeUnsafe(); } + // requires LockForIter + void EraseAndAdvance(Iterator &it) { extraObjects_.EraseAndAdvance(it); } + private: Queue extraObjects_; }; diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp index b7af6304abe..f946dc46060 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp @@ -52,7 +52,8 @@ TEST_F(ExtraObjectDataTest, Install) { EXPECT_FALSE(extraData.HasWeakReferenceCounter()); EXPECT_THAT(extraData.GetBaseObject(), object.header()); - mm::ExtraObjectData::Uninstall(object.header()); + extraData.Uninstall(); + mm::GlobalData::Instance().threadRegistry().CurrentThreadData()->ClearForTests(); EXPECT_FALSE(object.header()->has_meta_object()); EXPECT_THAT(object.header()->type_info(), typeInfo); diff --git a/kotlin-native/runtime/src/mm/cpp/Memory.cpp b/kotlin-native/runtime/src/mm/cpp/Memory.cpp index 488c29afba6..332783b77b3 100644 --- a/kotlin-native/runtime/src/mm/cpp/Memory.cpp +++ b/kotlin-native/runtime/src/mm/cpp/Memory.cpp @@ -82,7 +82,11 @@ MetaObjHeader* ObjHeader::createMetaObject(ObjHeader* object) { // static void ObjHeader::destroyMetaObject(ObjHeader* object) { - mm::ExtraObjectData::Uninstall(object); + RuntimeAssert(object->has_meta_object(), "Object must have a meta object set"); + auto &extraObject = *mm::ExtraObjectData::Get(object); + extraObject.Uninstall(); + auto *threadData = mm::ThreadRegistry::Instance().CurrentThreadData(); + mm::ExtraObjectDataFactory::Instance().DestroyExtraObjectData(threadData, extraObject); } ALWAYS_INLINE bool isPermanentOrFrozen(const ObjHeader* obj) {