diff --git a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp index 62fb9fb41a0..f3295450534 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp +++ b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp @@ -53,7 +53,7 @@ template typename Traits::ObjectFactory::FinalizerQueue Sweep(typename Traits::ObjectFactory& objectFactory) noexcept { typename Traits::ObjectFactory::FinalizerQueue finalizerQueue; - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (Traits::TryResetMark(*it)) { ++it; diff --git a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp index 2992eb2dfc4..fa2ce3d5c1c 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp +++ b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtilsSweepTest.cpp @@ -175,7 +175,7 @@ public: testing::Mock::VerifyAndClear(&finalizerHook()); // TODO: Figure out a better way to clear up the stuff. EXPECT_CALL(finalizerHook(), Call(testing::_)).Times(testing::AnyNumber()); - for (auto node : objectFactory_.Iter()) { + for (auto node : objectFactory_.LockForIter()) { auto* obj = node->IsArray() ? node->GetArrayHeader()->obj() : node->GetObjHeader(); if (auto* extraObject = mm::ExtraObjectData::Get(obj)) { extraObject->ClearWeakReferenceCounter(); @@ -196,7 +196,7 @@ public: KStdVector Alive() { KStdVector objects; - for (auto node : objectFactory_.Iter()) { + for (auto node : objectFactory_.LockForIter()) { objects.push_back(node.IsArray() ? node.GetArrayHeader()->obj() : node.GetObjHeader()); } return objects; diff --git a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp index 55b39adb147..2c4e33f04e1 100644 --- a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp +++ b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp @@ -126,7 +126,7 @@ mm::ObjectFactory::FinalizerQueue gc::SameThreadMark } KStdVector graySet; - for (auto& thread : mm::GlobalData::Instance().threadRegistry().Iter()) { + for (auto& thread : mm::GlobalData::Instance().threadRegistry().LockForIter()) { // TODO: Maybe it's more efficient to do by the suspending thread? thread.Publish(); for (auto* object : mm::ThreadRootSet(thread)) { diff --git a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp index bd361feca63..dc7359e7b20 100644 --- a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp +++ b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp @@ -191,7 +191,7 @@ KStdVector Alive(mm::ThreadData& threadData) { for (auto node : threadData.objectFactoryThreadQueue()) { objects.push_back(node.IsArray() ? node.GetArrayHeader()->obj() : node.GetObjHeader()); } - for (auto node : mm::GlobalData::Instance().objectFactory().Iter()) { + for (auto node : mm::GlobalData::Instance().objectFactory().LockForIter()) { objects.push_back(node.IsArray() ? node.GetArrayHeader()->obj() : node.GetObjHeader()); } return objects; diff --git a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp index fae3649807b..309730e8f52 100644 --- a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp +++ b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp @@ -119,7 +119,7 @@ public: // Lock `MultiSourceQueue` for safe iteration. If element was scheduled for deletion, // it'll still be iterated. Use `ApplyDeletions` to remove those elements. - Iterable Iter() noexcept { return Iterable(*this); } + Iterable LockForIter() noexcept { return Iterable(*this); } // Lock `MultiSourceQueue` and apply deletions. Only deletes elements that were published. void ApplyDeletions() noexcept { diff --git a/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp b/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp index 7f3e7a2c7af..40f060fed33 100644 --- a/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp +++ b/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp @@ -21,7 +21,7 @@ namespace { template KStdVector Collect(MultiSourceQueue& queue) { KStdVector result; - for (const auto& element : queue.Iter()) { + for (const auto& element : queue.LockForIter()) { result.push_back(element); } return result; @@ -254,7 +254,7 @@ TEST(MultiSourceQueueTest, IterWhileConcurrentPublish) { KStdVector actualBefore; { - auto iter = queue.Iter(); + auto iter = queue.LockForIter(); while (readyCount < kThreadCount) { } canStart = true; diff --git a/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp b/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp index 4f6508943c6..65b376fd7a5 100644 --- a/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp +++ b/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp @@ -71,7 +71,7 @@ public: class Iterable : private MoveOnly { public: - explicit Iterable(SingleLockList* list) noexcept : list_(list), guard_(list->mutex_) {} + explicit Iterable(SingleLockList* list) noexcept : list_(list), guard_(list->Lock()) {} Iterator begin() noexcept { return Iterator(list_->root_.get()); } @@ -136,12 +136,14 @@ public: // Returned value locks `this` to perform safe iteration. `this` unlocks when // `Iterable` gets out of scope. Example usage: - // for (auto& value: list.Iter()) { + // for (auto& value: list.LockForIter()) { // // Do something with `value`, there's a guarantee that it'll not be // // destroyed mid-iteration. // } // // At this point `list` is unlocked. - Iterable Iter() noexcept { return Iterable(this); } + Iterable LockForIter() noexcept { return Iterable(this); } + + std::unique_lock Lock() noexcept { return std::unique_lock(mutex_); } private: // Expects `mutex_` to be held by the current thread. diff --git a/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp b/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp index 91ac6e399e7..b576059dd43 100644 --- a/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp +++ b/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp @@ -49,7 +49,7 @@ TEST(SingleLockListTest, EmplaceAndIter) { list.Emplace(kThird); KStdVector actual; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actual.push_back(element); } @@ -67,7 +67,7 @@ TEST(SingleLockListTest, EmplaceEraseAndIter) { list.Erase(secondNode); KStdVector actual; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actual.push_back(element); } @@ -78,7 +78,7 @@ TEST(SingleLockListTest, IterEmpty) { IntList list; KStdVector actual; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actual.push_back(element); } @@ -99,7 +99,7 @@ TEST(SingleLockListTest, EraseToEmptyEmplaceAndIter) { list.Emplace(kFourth); KStdVector actual; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actual.push_back(element); } @@ -131,7 +131,7 @@ TEST(SingleLockListTest, ConcurrentEmplace) { } KStdVector actual; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actual.push_back(element); } @@ -166,7 +166,7 @@ TEST(SingleLockListTest, ConcurrentErase) { } KStdVector actual; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actual.push_back(element); } @@ -202,7 +202,7 @@ TEST(SingleLockListTest, IterWhileConcurrentEmplace) { KStdVector actualBefore; { - auto iter = list.Iter(); + auto iter = list.LockForIter(); canStart = true; while (startedCount < kThreadCount) { } @@ -219,7 +219,7 @@ TEST(SingleLockListTest, IterWhileConcurrentEmplace) { EXPECT_THAT(actualBefore, testing::ElementsAreArray(expectedBefore)); KStdVector actualAfter; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actualAfter.push_back(element); } @@ -251,7 +251,7 @@ TEST(SingleLockListTest, IterWhileConcurrentErase) { KStdVector actualBefore; { - auto iter = list.Iter(); + auto iter = list.LockForIter(); canStart = true; while (startedCount < kThreadCount) { } @@ -268,13 +268,99 @@ TEST(SingleLockListTest, IterWhileConcurrentErase) { EXPECT_THAT(actualBefore, testing::ElementsAreArray(expectedBefore)); KStdVector actualAfter; - for (int element : list.Iter()) { + for (int element : list.LockForIter()) { actualAfter.push_back(element); } EXPECT_THAT(actualAfter, testing::IsEmpty()); } +TEST(SingleLockListTest, LockAndEmplace) { + SingleLockList list; + constexpr int kThreadCount = kDefaultThreadCount; + + KStdVector threads; + KStdVector actualLocked; + KStdVector actualUnlocked; + KStdVector expectedUnlocked; + for (int i = 0; i < kThreadCount; i++) { + expectedUnlocked.push_back(i); + } + std::atomic startedCount(0); + { + std::unique_lock lock = list.Lock(); + for (int i = 0; i < kThreadCount; i++) { + threads.emplace_back([&startedCount, &list, i]() { + startedCount++; + list.Emplace(i); + }); + } + while (startedCount != kThreadCount) { + std::this_thread::yield(); + } + // Here still may be a race leading to false-successful EXPECT + // if the scheduler suspend all threads right before list.Emplace. + // But this situation looks unlikely + for (int element : list.LockForIter()) { + actualLocked.push_back(element); + } + } + + for (auto& thread : threads) { + thread.join(); + } + for (int element : list.LockForIter()) { + actualUnlocked.push_back(element); + } + EXPECT_THAT(actualLocked, testing::IsEmpty()); + EXPECT_THAT(actualUnlocked, testing::UnorderedElementsAreArray(expectedUnlocked)); +} + +TEST(SingleLockListTest, LockAndErase) { + SingleLockList list; + constexpr int kThreadCount = kDefaultThreadCount; + + KStdVector::Node*> items; + KStdVector expectedLocked; + KStdVector threads; + KStdVector actualLocked; + KStdVector actualUnlocked; + std::atomic startedCount(0); + + for (int i = 0; i < kThreadCount; i++) { + expectedLocked.push_back(i); + items.push_back(list.Emplace(i)); + } + { + std::unique_lock lock = list.Lock(); + for (int i = 0; i < kThreadCount; i++) { + threads.emplace_back([&startedCount, &list, &items, i]() { + startedCount++; + list.Erase(items[i]); + }); + } + while (startedCount != kThreadCount) { + std::this_thread::yield(); + } + // Here still may be a race leading to false-successful EXPECT + // if the scheduler suspend all threads right before list.Erase. + // But this situation looks unlikely + for (int element : list.LockForIter()) { + actualLocked.push_back(element); + } + } + + for (auto& thread : threads) { + thread.join(); + } + for (int element : list.LockForIter()) { + actualUnlocked.push_back(element); + } + + EXPECT_THAT(actualLocked, testing::UnorderedElementsAreArray(expectedLocked)); + EXPECT_THAT(actualUnlocked, testing::IsEmpty()); +} + namespace { class PinnedType : private Pinned { @@ -300,7 +386,7 @@ TEST(SingleLockListTest, PinnedType) { list.Erase(itemNode); KStdVector actualAfter; - for (auto& element : list.Iter()) { + for (auto& element : list.LockForIter()) { actualAfter.push_back(&element); } diff --git a/kotlin-native/runtime/src/mm/cpp/ExceptionObjHolderTest.cpp b/kotlin-native/runtime/src/mm/cpp/ExceptionObjHolderTest.cpp index 7fd91b07634..dfbb751331b 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExceptionObjHolderTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ExceptionObjHolderTest.cpp @@ -24,7 +24,7 @@ public: stableRefs.ProcessThread(&threadData); stableRefs.ProcessDeletions(); KStdVector result; - for (const auto& obj : stableRefs.Iter()) { + for (const auto& obj : stableRefs.LockForIter()) { result.push_back(obj); } return result; diff --git a/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp b/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp index ce0177eea0f..e06a4e5ef70 100644 --- a/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp +++ b/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp @@ -41,7 +41,7 @@ public: // TODO: Iteration over `globals_` will be slow, because it's `KStdList` collected at different times from // different threads, and so the nodes are all over the memory. Use metrics to understand how // much of a problem is it. - Iterable Iter() noexcept { return globals_.Iter(); } + Iterable LockForIter() noexcept { return globals_.LockForIter(); } void ClearForTests() { globals_.ClearForTests(); } diff --git a/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp b/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp index 78b2d11e4f8..4352e159253 100644 --- a/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp @@ -324,7 +324,7 @@ public: } // Lock `ObjectFactoryStorage` for safe iteration. - Iterable Iter() noexcept { return Iterable(*this); } + Iterable LockForIter() noexcept { return Iterable(*this); } void ClearForTests() { root_.reset(); @@ -619,7 +619,7 @@ public: class Iterable { public: - Iterable(ObjectFactory& owner) noexcept : iter_(owner.storage_.Iter()) {} + Iterable(ObjectFactory& owner) noexcept : iter_(owner.storage_.LockForIter()) {} Iterator begin() noexcept { return Iterator(iter_.begin()); } Iterator end() noexcept { return Iterator(iter_.end()); } @@ -637,7 +637,8 @@ public: ObjectFactory() noexcept = default; ~ObjectFactory() = default; - Iterable Iter() noexcept { return Iterable(*this); } + // Lock ObjectFactory for safe iteration. + Iterable LockForIter() noexcept { return Iterable(*this); } void ClearForTests() { storage_.ClearForTests(); } diff --git a/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp b/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp index 5f619c6da1e..73da69a7ce1 100644 --- a/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp @@ -39,7 +39,7 @@ using Consumer = typename Storage::Consumer; template KStdVector Collect(ObjectFactoryStorage& storage) { KStdVector result; - for (auto& node : storage.Iter()) { + for (auto& node : storage.LockForIter()) { result.push_back(node.Data()); } return result; @@ -48,7 +48,7 @@ KStdVector Collect(ObjectFactoryStorage& storage) { template KStdVector Collect(ObjectFactoryStorage& storage) { KStdVector result; - for (auto& node : storage.Iter()) { + for (auto& node : storage.LockForIter()) { result.push_back(*static_cast(node.Data())); } return result; @@ -137,7 +137,7 @@ TEST(ObjectFactoryStorageTest, PublishDifferentTypes) { producer.Publish(); - auto actual = storage.Iter(); + auto actual = storage.LockForIter(); auto it = actual.begin(); EXPECT_THAT(it->Data(), 1); ++it; @@ -222,7 +222,7 @@ TEST(ObjectFactoryStorageTest, EraseFirst) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->Data() == 1) { iter.EraseAndAdvance(it); @@ -248,7 +248,7 @@ TEST(ObjectFactoryStorageTest, EraseMiddle) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->Data() == 2) { iter.EraseAndAdvance(it); @@ -274,7 +274,7 @@ TEST(ObjectFactoryStorageTest, EraseLast) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->Data() == 3) { iter.EraseAndAdvance(it); @@ -300,7 +300,7 @@ TEST(ObjectFactoryStorageTest, EraseAll) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { iter.EraseAndAdvance(it); } @@ -320,7 +320,7 @@ TEST(ObjectFactoryStorageTest, EraseTheOnlyElement) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); auto it = iter.begin(); iter.EraseAndAdvance(it); EXPECT_THAT(it, iter.end()); @@ -343,7 +343,7 @@ TEST(ObjectFactoryStorageTest, MoveFirst) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->Data() == 1) { iter.MoveAndAdvance(consumer, it); @@ -372,7 +372,7 @@ TEST(ObjectFactoryStorageTest, MoveMiddle) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->Data() == 2) { iter.MoveAndAdvance(consumer, it); @@ -401,7 +401,7 @@ TEST(ObjectFactoryStorageTest, MoveLast) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->Data() == 3) { iter.MoveAndAdvance(consumer, it); @@ -430,7 +430,7 @@ TEST(ObjectFactoryStorageTest, MoveAll) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { iter.MoveAndAdvance(consumer, it); } @@ -453,7 +453,7 @@ TEST(ObjectFactoryStorageTest, MoveTheOnlyElement) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); auto it = iter.begin(); iter.MoveAndAdvance(consumer, it); EXPECT_THAT(it, iter.end()); @@ -484,7 +484,7 @@ TEST(ObjectFactoryStorageTest, MoveAndErase) { producer.Publish(); { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { ++it; iter.EraseAndAdvance(it); @@ -566,7 +566,7 @@ TEST(ObjectFactoryStorageTest, IterWhileConcurrentPublish) { KStdVector actualBefore; { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); while (readyCount < kThreadCount) { } canStart = true; @@ -624,7 +624,7 @@ TEST(ObjectFactoryStorageTest, EraseWhileConcurrentPublish) { } { - auto iter = storage.Iter(); + auto iter = storage.LockForIter(); while (readyCount < kThreadCount) { } canStart = true; @@ -772,7 +772,7 @@ TEST(ObjectFactoryTest, CreateObject) { EXPECT_THAT(node.GetObjHeader(), object); EXPECT_THAT(node.GCObjectData().flags, 42); - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); auto it = iter.begin(); EXPECT_THAT(*it, node); ++it; @@ -792,7 +792,7 @@ TEST(ObjectFactoryTest, CreateObjectArray) { EXPECT_THAT(node.GetArrayHeader(), array); EXPECT_THAT(node.GCObjectData().flags, 42); - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); auto it = iter.begin(); EXPECT_THAT(*it, node); ++it; @@ -812,7 +812,7 @@ TEST(ObjectFactoryTest, CreateCharArray) { EXPECT_THAT(node.GetArrayHeader(), array); EXPECT_THAT(node.GCObjectData().flags, 42); - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); auto it = iter.begin(); EXPECT_THAT(*it, node); ++it; @@ -833,7 +833,7 @@ TEST(ObjectFactoryTest, Erase) { threadQueue.Publish(); { - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->IsArray()) { iter.EraseAndAdvance(it); @@ -844,7 +844,7 @@ TEST(ObjectFactoryTest, Erase) { } { - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); int count = 0; for (auto it = iter.begin(); it != iter.end(); ++it, ++count) { EXPECT_FALSE(it->IsArray()); @@ -868,7 +868,7 @@ TEST(ObjectFactoryTest, Move) { threadQueue.Publish(); { - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { if (it->IsArray()) { iter.MoveAndAdvance(finalizerQueue, it); @@ -879,7 +879,7 @@ TEST(ObjectFactoryTest, Move) { } { - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); int count = 0; for (auto it = iter.begin(); it != iter.end(); ++it, ++count) { EXPECT_FALSE(it->IsArray()); @@ -914,7 +914,7 @@ TEST(ObjectFactoryTest, RunFinalizers) { threadQueue.Publish(); { - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); for (auto it = iter.begin(); it != iter.end();) { iter.MoveAndAdvance(finalizerQueue, it); } @@ -961,7 +961,7 @@ TEST(ObjectFactoryTest, ConcurrentPublish) { t.join(); } - auto iter = objectFactory.Iter(); + auto iter = objectFactory.LockForIter(); KStdVector actual; for (auto it = iter.begin(); it != iter.end(); ++it) { actual.push_back(it->GetObjHeader()); diff --git a/kotlin-native/runtime/src/mm/cpp/RootSet.hpp b/kotlin-native/runtime/src/mm/cpp/RootSet.hpp index 219d9a39f8c..dcd5c2178da 100644 --- a/kotlin-native/runtime/src/mm/cpp/RootSet.hpp +++ b/kotlin-native/runtime/src/mm/cpp/RootSet.hpp @@ -104,7 +104,7 @@ public: }; GlobalRootSet(GlobalsRegistry& globalsRegistry, StableRefRegistry& stableRefRegistry) noexcept : - globalsIterable_(globalsRegistry.Iter()), stableRefsIterable_(stableRefRegistry.Iter()) {} + globalsIterable_(globalsRegistry.LockForIter()), stableRefsIterable_(stableRefRegistry.LockForIter()) {} GlobalRootSet() noexcept; Iterator begin() noexcept { return Iterator(Iterator::begin, *this); } diff --git a/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp b/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp index ec272554d4e..399197f38e5 100644 --- a/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp +++ b/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp @@ -39,14 +39,14 @@ public: // when it's asked by GC to stop. void ProcessThread(mm::ThreadData* threadData) noexcept; - // Lock registry and apply deletions. Should be called on GC thread after all threads have published, and before `Iter`. + // Lock registry and apply deletions. Should be called on GC thread after all threads have published, and before `LockForIter`. void ProcessDeletions() noexcept; // Lock registry for safe iteration. // TODO: Iteration over `stableRefs_` will be slow, because it's `KStdList` collected at different times from // different threads, and so the nodes are all over the memory. Use metrics to understand how // much of a problem is it. - Iterable Iter() noexcept { return stableRefs_.Iter(); } + Iterable LockForIter() noexcept { return stableRefs_.LockForIter(); } void ClearForTests() noexcept { stableRefs_.ClearForTests(); } diff --git a/kotlin-native/runtime/src/mm/cpp/TestSupport.cpp b/kotlin-native/runtime/src/mm/cpp/TestSupport.cpp index db374abad28..faf2884cd69 100644 --- a/kotlin-native/runtime/src/mm/cpp/TestSupport.cpp +++ b/kotlin-native/runtime/src/mm/cpp/TestSupport.cpp @@ -44,10 +44,10 @@ std::vector collect(mm::ThreadRegistry::Iterable& iterable) { extern "C" void Kotlin_TestSupport_AssertClearGlobalState() { // Validate that global registries are empty. - auto globals = mm::GlobalsRegistry::Instance().Iter(); - auto objects = mm::GlobalData::Instance().objectFactory().Iter(); - auto stableRefs = mm::StableRefRegistry::Instance().Iter(); - auto threads = mm::ThreadRegistry::Instance().Iter(); + auto globals = mm::GlobalsRegistry::Instance().LockForIter(); + auto objects = mm::GlobalData::Instance().objectFactory().LockForIter(); + auto stableRefs = mm::StableRefRegistry::Instance().LockForIter(); + auto threads = mm::ThreadRegistry::Instance().LockForIter(); EXPECT_THAT(collect(globals), testing::UnorderedElementsAre()); EXPECT_THAT(collect::NodeRef>(objects), testing::UnorderedElementsAre()); diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.cpp index 068fe62583b..2f4b44cadd1 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.cpp @@ -31,8 +31,12 @@ void mm::ThreadRegistry::Unregister(Node* threadDataNode) noexcept { // Do not touch `currentThreadData_` as TLS may already have been deallocated. } -mm::ThreadRegistry::Iterable mm::ThreadRegistry::Iter() noexcept { - return list_.Iter(); +mm::ThreadRegistry::Iterable mm::ThreadRegistry::LockForIter() noexcept { + return list_.LockForIter(); +} + +std::unique_lock mm::ThreadRegistry::Lock() noexcept { + return list_.Lock(); } ALWAYS_INLINE mm::ThreadData* mm::ThreadRegistry::CurrentThreadData() const noexcept { diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.hpp b/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.hpp index 00e2204d7e1..7eac82a73c5 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadRegistry.hpp @@ -19,8 +19,9 @@ class ThreadData; class ThreadRegistry final : private Pinned { public: - using Node = SingleLockList::Node; - using Iterable = SingleLockList::Iterable; + using Mutex = std::recursive_mutex; + using Node = SingleLockList::Node; + using Iterable = SingleLockList::Iterable; static ThreadRegistry& Instance() noexcept; @@ -30,7 +31,9 @@ public: void Unregister(Node* threadDataNode) noexcept; // Locks `ThreadRegistry` for safe iteration. - Iterable Iter() noexcept; + Iterable LockForIter() noexcept; + + std::unique_lock Lock() noexcept; // Try not to use these methods very often, as (1) thread local access can be slow on some platforms, // (2) TLS gets deallocated before our thread destruction hooks run. @@ -51,7 +54,7 @@ private: static THREAD_LOCAL_VARIABLE Node* currentThreadDataNode_; - SingleLockList list_; + SingleLockList list_; }; } // namespace mm diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp index 424c61c35d2..cd2af06523c 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp @@ -21,7 +21,7 @@ template bool allThreads(F predicate) noexcept { auto& threadRegistry = kotlin::mm::ThreadRegistry::Instance(); auto* currentThread = threadRegistry.CurrentThreadData(); - kotlin::mm::ThreadRegistry::Iterable threads = kotlin::mm::ThreadRegistry::Instance().Iter(); + kotlin::mm::ThreadRegistry::Iterable threads = kotlin::mm::ThreadRegistry::Instance().LockForIter(); for (auto& thread : threads) { // Handle if suspension was initiated by the mutator thread. if (&thread == currentThread) diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp index 4d53f66c52f..a0ffc5530b2 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp @@ -29,7 +29,7 @@ constexpr size_t kDefaultReportingStep = 1000; KStdVector collectThreadData() { KStdVector result; - auto iter = mm::ThreadRegistry::Instance().Iter(); + auto iter = mm::ThreadRegistry::Instance().LockForIter(); for (auto& thread : iter) { result.push_back(&thread); }