[K/N] Make GC.collect wait for finalizers being run

^KT-50713

Merge-request: KT-MR-5404
Merged-by: Alexander Shabalin <Alexander.Shabalin@jetbrains.com>

kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp

@@ -75,10 +75,6 @@ void gc::ConcurrentMarkAndSweep::ThreadData::ScheduleAndWaitFullGCWithFinalizers
     gc_.state_.waitEpochFinalized(scheduled_epoch);
 }
 
-void gc::ConcurrentMarkAndSweep::ThreadData::StopFinalizerThreadForTests() noexcept {
-    gc_.finalizerProcessor_->StopFinalizerThread();
-}
-
 void gc::ConcurrentMarkAndSweep::ThreadData::OnOOM(size_t size) noexcept {
     RuntimeLogDebug({kTagGC}, "Attempt to GC on OOM at size=%zu", size);
     ScheduleAndWaitFullGC();
@@ -110,6 +106,21 @@ gc::ConcurrentMarkAndSweep::~ConcurrentMarkAndSweep() {
     gcThread_.join();
 }
 
+void gc::ConcurrentMarkAndSweep::StartFinalizerThreadIfNeeded() noexcept {
+    NativeOrUnregisteredThreadGuard guard(true);
+    finalizerProcessor_->StartFinalizerThreadIfNone();
+    finalizerProcessor_->WaitFinalizerThreadInitialized();
+}
+
+void gc::ConcurrentMarkAndSweep::StopFinalizerThreadIfRunning() noexcept {
+    NativeOrUnregisteredThreadGuard guard(true);
+    finalizerProcessor_->StopFinalizerThread();
+}
+
+bool gc::ConcurrentMarkAndSweep::FinalizersThreadIsRunning() noexcept {
+    return finalizerProcessor_->IsRunning();
+}
+
 bool gc::ConcurrentMarkAndSweep::PerformFullGC(int64_t epoch) noexcept {
     RuntimeLogDebug({kTagGC}, "Attempt to suspend threads by thread %d", konan::currentThreadId());
     auto timeStartUs = konan::getTimeMicros();

kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.hpp

@@ -57,7 +57,6 @@ public:
 
         void ScheduleAndWaitFullGC() noexcept;
         void ScheduleAndWaitFullGCWithFinalizers() noexcept;
-        void StopFinalizerThreadForTests() noexcept;
 
         void OnOOM(size_t size) noexcept;
 
@@ -73,6 +72,10 @@ public:
     ConcurrentMarkAndSweep(mm::ObjectFactory<ConcurrentMarkAndSweep>& objectFactory, GCScheduler& scheduler) noexcept;
     ~ConcurrentMarkAndSweep();
 
+    void StartFinalizerThreadIfNeeded() noexcept;
+    void StopFinalizerThreadIfRunning() noexcept;
+    bool FinalizersThreadIsRunning() noexcept;
+
 private:
     // Returns `true` if GC has happened, and `false` if not (because someone else has suspended the threads).
     bool PerformFullGC(int64_t epoch) noexcept;

kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp

@@ -340,7 +340,6 @@ TEST_F(ConcurrentMarkAndSweepTest, FreeObjectsWithFinalizers) {
         EXPECT_CALL(finalizerHook(), Call(object1.header()));
         EXPECT_CALL(finalizerHook(), Call(object2.header()));
         threadData.gc().ScheduleAndWaitFullGCWithFinalizers();
-        threadData.gc().impl().gc().StopFinalizerThreadForTests();
 
         EXPECT_THAT(Alive(threadData), testing::UnorderedElementsAre());
     });
@@ -360,7 +359,6 @@ TEST_F(ConcurrentMarkAndSweepTest, FreeObjectWithFreeWeak) {
         ASSERT_THAT(weak1->referred, object1.header());
 
         threadData.gc().ScheduleAndWaitFullGCWithFinalizers();
-        threadData.gc().impl().gc().StopFinalizerThreadForTests();
 
         EXPECT_THAT(Alive(threadData), testing::UnorderedElementsAre());
     });
@@ -511,7 +509,6 @@ TEST_F(ConcurrentMarkAndSweepTest, ObjectsWithCyclesAndFinalizers) {
         EXPECT_CALL(finalizerHook(), Call(object5.header()));
         EXPECT_CALL(finalizerHook(), Call(object6.header()));
         threadData.gc().ScheduleAndWaitFullGCWithFinalizers();
-        threadData.gc().impl().gc().StopFinalizerThreadForTests();
 
         EXPECT_THAT(
                 Alive(threadData),

kotlin-native/runtime/src/gc/cms/cpp/FinalizerProcessor.cpp

@@ -14,6 +14,11 @@ void kotlin::gc::FinalizerProcessor::StartFinalizerThreadIfNone() noexcept {
     if (finalizerThread_.joinable()) return;
     finalizerThread_ = std::thread([this] {
         Kotlin_initRuntimeIfNeeded();
+        {
+            std::unique_lock guard(initializedMutex_);
+            initialized_ = true;
+        }
+        initializedCondVar_.notify_all();
         int64_t finalizersEpoch = 0;
         while (true) {
             std::unique_lock lock(finalizerQueueMutex_);
@@ -34,6 +39,11 @@ void kotlin::gc::FinalizerProcessor::StartFinalizerThreadIfNone() noexcept {
             }
             epochDoneCallback_(finalizersEpoch);
         }
+        {
+            std::unique_lock guard(initializedMutex_);
+            initialized_ = false;
+        }
+        initializedCondVar_.notify_all();
     });
 }
 
@@ -69,6 +79,11 @@ bool kotlin::gc::FinalizerProcessor::IsRunning() noexcept {
     return finalizerThread_.joinable();
 }
 
+void kotlin::gc::FinalizerProcessor::WaitFinalizerThreadInitialized() noexcept {
+    std::unique_lock guard(initializedMutex_);
+    initializedCondVar_.wait(guard, [this] { return initialized_; });
+}
+
 kotlin::gc::FinalizerProcessor::~FinalizerProcessor() {
     StopFinalizerThread();
-}
+}

kotlin-native/runtime/src/gc/cms/cpp/FinalizerProcessor.hpp

@@ -10,6 +10,7 @@
 #include "GCState.hpp"
 
 namespace kotlin::gc {
+
 class FinalizerProcessor : Pinned {
 public:
     using Queue = typename kotlin::mm::ObjectFactory<ConcurrentMarkAndSweep>::FinalizerQueue;
@@ -19,9 +20,11 @@ public:
     void ScheduleTasks(Queue&& tasks, int64_t epoch) noexcept;
     void StopFinalizerThread() noexcept;
     bool IsRunning() noexcept;
-    ~FinalizerProcessor();
-private:
     void StartFinalizerThreadIfNone() noexcept;
+    void WaitFinalizerThreadInitialized() noexcept;
+    ~FinalizerProcessor();
+
+private:
     std::thread finalizerThread_;
     Queue finalizerQueue_;
     std::condition_variable finalizerQueueCondVar_;
@@ -30,5 +33,10 @@ private:
     int64_t finalizerQueueEpoch_ = 0;
     bool shutdownFlag_ = false;
     bool newTasksAllowed_ = true;
+
+    std::mutex initializedMutex_;
+    std::condition_variable initializedCondVar_;
+    bool initialized_ = false;
 };
-}
+
+} // namespace kotlin::gc

kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp

@@ -73,5 +73,18 @@ gc::GCSchedulerConfig& gc::GC::gcSchedulerConfig() noexcept {
 }
 
 void gc::GC::ClearForTests() noexcept {
+    impl_->gc().StopFinalizerThreadIfRunning();
     impl_->objectFactory().ClearForTests();
 }
+
+void gc::GC::StartFinalizerThreadIfNeeded() noexcept {
+    impl_->gc().StartFinalizerThreadIfNeeded();
+}
+
+void gc::GC::StopFinalizerThreadIfRunning() noexcept {
+    impl_->gc().StopFinalizerThreadIfRunning();
+}
+
+bool gc::GC::FinalizersThreadIsRunning() noexcept {
+    return impl_->gc().FinalizersThreadIsRunning();
+}

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

@@ -60,6 +60,10 @@ public:
 
     void ClearForTests() noexcept;
 
+    void StartFinalizerThreadIfNeeded() noexcept;
+    void StopFinalizerThreadIfRunning() noexcept;
+    bool FinalizersThreadIsRunning() noexcept;
+
 private:
     KStdUniquePtr<Impl> impl_;
 };

kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp

@@ -65,3 +65,11 @@ gc::GCSchedulerConfig& gc::GC::gcSchedulerConfig() noexcept {
 void gc::GC::ClearForTests() noexcept {
     impl_->objectFactory().ClearForTests();
 }
+
+void gc::GC::StartFinalizerThreadIfNeeded() noexcept {}
+
+void gc::GC::StopFinalizerThreadIfRunning() noexcept {}
+
+bool gc::GC::FinalizersThreadIsRunning() noexcept {
+    return false;
+}

kotlin-native/runtime/src/gc/noop/cpp/NoOpGC.hpp

@@ -55,7 +55,6 @@ public:
     ~NoOpGC() = default;
 
     GCScheduler& scheduler() noexcept { return scheduler_; }
-    void StopFinalizerThreadForTests() noexcept {}
 
 private:
     GCScheduler scheduler_;

kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp

@@ -77,3 +77,11 @@ gc::GCSchedulerConfig& gc::GC::gcSchedulerConfig() noexcept {
 void gc::GC::ClearForTests() noexcept {
     impl_->objectFactory().ClearForTests();
 }
+
+void gc::GC::StartFinalizerThreadIfNeeded() noexcept {}
+
+void gc::GC::StopFinalizerThreadIfRunning() noexcept {}
+
+bool gc::GC::FinalizersThreadIsRunning() noexcept {
+    return false;
+}

kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.hpp

@@ -75,7 +75,6 @@ public:
 
     SameThreadMarkAndSweep(mm::ObjectFactory<SameThreadMarkAndSweep>& objectFactory, GCScheduler& gcScheduler) noexcept;
     ~SameThreadMarkAndSweep() = default;
-    void StopFinalizerThreadForTests() noexcept {}
 
 private:
     // Returns `true` if GC has happened, and `false` if not (because someone else has suspended the threads).

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

@@ -3803,3 +3803,9 @@ ALWAYS_INLINE kotlin::CalledFromNativeGuard::CalledFromNativeGuard(bool reentran
 }
 
 const bool kotlin::kSupportsMultipleMutators = true;
+
+void kotlin::StartFinalizerThreadIfNeeded() noexcept {}
+
+bool kotlin::FinalizersThreadIsRunning() noexcept {
+    return false;
+}

kotlin-native/runtime/src/main/cpp/Memory.h

@@ -520,6 +520,9 @@ private:
 
 extern const bool kSupportsMultipleMutators;
 
+void StartFinalizerThreadIfNeeded() noexcept;
+bool FinalizersThreadIsRunning() noexcept;
+
 } // namespace kotlin
 
 #endif // RUNTIME_MEMORY_H

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

@@ -243,6 +243,10 @@ void Kotlin_shutdownRuntime() {
         return;
     }
 
+    // If we're going to need finalizers for the full shutdown, we need to start the thread before
+    // new runtimes are disallowed.
+    kotlin::StartFinalizerThreadIfNeeded();
+
     if (Kotlin_cleanersLeakCheckerEnabled()) {
         // Make sure to collect any lingering cleaners.
         PerformFullGC(runtime->memoryState);
@@ -262,8 +266,14 @@ void Kotlin_shutdownRuntime() {
         // First make sure workers are gone.
         WaitNativeWorkersTermination();
 
+        // Allow the current runtime.
+        int knownRuntimes = 1;
+        if (kotlin::FinalizersThreadIsRunning()) {
+            ++knownRuntimes;
+        }
+
         // Now check for existence of any other runtimes.
-        auto otherRuntimesCount = atomicGet(&aliveRuntimesCount) - 1;
+        auto otherRuntimesCount = atomicGet(&aliveRuntimesCount) - knownRuntimes;
         RuntimeAssert(otherRuntimesCount >= 0, "Cannot be negative");
         if (Kotlin_forceCheckedShutdown()) {
             if (otherRuntimesCount > 0) {

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

@@ -111,8 +111,9 @@ extern "C" void DeinitMemory(MemoryState* state, bool destroyRuntime) {
     auto* node = mm::FromMemoryState(state);
     if (destroyRuntime) {
         ThreadStateGuard guard(state, ThreadState::kRunnable);
-        node->Get()->gc().ScheduleAndWaitFullGC();
+        node->Get()->gc().ScheduleAndWaitFullGCWithFinalizers();
-        // TODO: Also make sure that finalizers are run.
+        // TODO: Why not just destruct `GC` object and its thread data counterpart entirely?
+        mm::GlobalData::Instance().gc().StopFinalizerThreadIfRunning();
     }
     mm::ThreadRegistry::Instance().Unregister(node);
     if (destroyRuntime) {
@@ -293,7 +294,7 @@ extern "C" RUNTIME_NOTHROW void GC_CollectorCallback(void* worker) {
 
 extern "C" void Kotlin_native_internal_GC_collect(ObjHeader*) {
     auto* threadData = mm::ThreadRegistry::Instance().CurrentThreadData();
-    threadData->gc().ScheduleAndWaitFullGC();
+    threadData->gc().ScheduleAndWaitFullGCWithFinalizers();
 }
 
 extern "C" void Kotlin_native_internal_GC_collectCyclic(ObjHeader*) {
@@ -402,7 +403,7 @@ extern "C" void Kotlin_Any_share(ObjHeader* thiz) {
 }
 
 extern "C" RUNTIME_NOTHROW void PerformFullGC(MemoryState* memory) {
-    memory->GetThreadData()->gc().ScheduleAndWaitFullGC();
+    memory->GetThreadData()->gc().ScheduleAndWaitFullGCWithFinalizers();
 }
 
 extern "C" bool TryAddHeapRef(const ObjHeader* object) {
@@ -572,3 +573,11 @@ ALWAYS_INLINE kotlin::CalledFromNativeGuard::CalledFromNativeGuard(bool reentran
 }
 
 const bool kotlin::kSupportsMultipleMutators = kotlin::gc::kSupportsMultipleMutators;
+
+void kotlin::StartFinalizerThreadIfNeeded() noexcept {
+    mm::GlobalData::Instance().gc().StartFinalizerThreadIfNeeded();
+}
+
+bool kotlin::FinalizersThreadIsRunning() noexcept {
+    return mm::GlobalData::Instance().gc().FinalizersThreadIsRunning();
+}