From 42aaa86f10a85d4623944980803505000bde3d6d Mon Sep 17 00:00:00 2001
From: Pavel Kunyavskiy <pavel.kunyavskiy@jetbrains.com>
Date: Fri, 3 Dec 2021 16:31:23 +0300
Subject: [PATCH] [K/N] Mark finalizing extra objects to avoid double
 finalizing

When new mark phase finished before previous finalization is done,
extra objects are still in list, which would be passed to finalize.
This could lead to unpredictable effects.

After this commit such objects are marked by new flag and ignored by
next gc runs.
---
 .../runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp |  3 ++-
 .../runtime/src/mm/cpp/ExtraObjectData.hpp          | 13 ++++++++-----
 kotlin-native/runtime/src/mm/cpp/Freezing.cpp       | 12 ++++++------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp
index 48b73075c09..dbb67c3b365 100644
--- a/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp
+++ b/kotlin-native/runtime/src/gc/common/cpp/MarkAndSweepUtils.hpp
@@ -69,10 +69,11 @@ void SweepExtraObjects(typename Traits::ExtraObjectsFactory& objectFactory) noex
     auto iter = objectFactory.LockForIter();
     for (auto it = iter.begin(); it != iter.end();) {
         auto &extraObject = *it;
-        if (!Traits::IsMarkedByExtraObject(extraObject)) {
+        if (!extraObject.getFlag(mm::ExtraObjectData::FLAGS_IN_FINALIZER_QUEUE) && !Traits::IsMarkedByExtraObject(extraObject)) {
             extraObject.ClearWeakReferenceCounter();
             if (extraObject.HasAssociatedObject()) {
                 extraObject.DetachAssociatedObject();
+                extraObject.setFlag(mm::ExtraObjectData::FLAGS_IN_FINALIZER_QUEUE);
                 ++it;
             } else {
                 extraObject.Uninstall();
diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp
index 982891b51d2..0714954a888 100644
--- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp
+++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp
@@ -23,10 +23,11 @@ namespace mm {
 // Optional data that's lazily allocated only for objects that need it.
 class ExtraObjectData : private Pinned, public KonanAllocatorAware {
 public:
+    // flags are stored as single atomic uint32, values are bit numbers in that uint32
     enum Flags : uint32_t {
-        FLAGS_NONE = 0,
-        FLAGS_FROZEN = 1 << 0,
-        FLAGS_NEVER_FROZEN = 1 << 1,
+        FLAGS_FROZEN = 0,
+        FLAGS_NEVER_FROZEN = 1,
+        FLAGS_IN_FINALIZER_QUEUE = 2,
     };
 
     static constexpr unsigned WEAK_REF_TAG = 1;
@@ -49,7 +50,9 @@ public:
     bool HasAssociatedObject() noexcept;
     void DetachAssociatedObject() noexcept;
 
-    std::atomic<Flags>& flags() noexcept { return flags_; }
+    bool getFlag(Flags value) noexcept { return (flags_.load() & (1u << static_cast<uint32_t>(value))) != 0; }
+    void setFlag(Flags value) noexcept { flags_.fetch_or(1u << static_cast<uint32_t>(value)); }
+
 
     bool HasWeakReferenceCounter() noexcept { return hasPointerBits(weakReferenceCounterOrBaseObject_.load(), WEAK_REF_TAG); }
     void ClearWeakReferenceCounter() noexcept;
@@ -84,7 +87,7 @@ private:
     // Must be first to match `TypeInfo` layout.
     const TypeInfo* typeInfo_;
 
-    std::atomic<Flags> flags_ = FLAGS_NONE;
+    std::atomic<uint32_t> flags_ = 0;
 
 #ifdef KONAN_OBJC_INTEROP
     void* associatedObject_ = nullptr;
diff --git a/kotlin-native/runtime/src/mm/cpp/Freezing.cpp b/kotlin-native/runtime/src/mm/cpp/Freezing.cpp
index f696f510902..6c8ecfd2287 100644
--- a/kotlin-native/runtime/src/mm/cpp/Freezing.cpp
+++ b/kotlin-native/runtime/src/mm/cpp/Freezing.cpp
@@ -20,7 +20,7 @@ bool mm::IsFrozen(const ObjHeader* object) noexcept {
     }
 
     if (auto* extraObjectData = mm::ExtraObjectData::Get(object)) {
-        return (extraObjectData->flags() & mm::ExtraObjectData::FLAGS_FROZEN) != 0;
+        return extraObjectData->getFlag(mm::ExtraObjectData::FLAGS_FROZEN);
     }
     return false;
 }
@@ -48,12 +48,13 @@ ObjHeader* mm::FreezeSubgraph(ObjHeader* root) noexcept {
     }
     for (auto* object : objects) {
         if (auto* extraObjectData = mm::ExtraObjectData::Get(object)) {
-            if ((extraObjectData->flags() & mm::ExtraObjectData::FLAGS_NEVER_FROZEN) != 0) return object;
+            if (extraObjectData->getFlag(mm::ExtraObjectData::FLAGS_NEVER_FROZEN)) {
+                return object;
+            }
         }
     }
     for (auto* object : objects) {
-        auto& flags = mm::ExtraObjectData::GetOrInstall(object).flags();
-        flags = static_cast<mm::ExtraObjectData::Flags>(flags | mm::ExtraObjectData::FLAGS_FROZEN);
+        mm::ExtraObjectData::GetOrInstall(object).setFlag(mm::ExtraObjectData::FLAGS_FROZEN);
     }
     return nullptr;
 }
@@ -63,7 +64,6 @@ bool mm::EnsureNeverFrozen(ObjHeader* object) noexcept {
         return false;
     }
 
-    auto& flags = mm::ExtraObjectData::GetOrInstall(object).flags();
-    flags = static_cast<mm::ExtraObjectData::Flags>(flags | mm::ExtraObjectData::FLAGS_NEVER_FROZEN);
+    mm::ExtraObjectData::GetOrInstall(object).setFlag(mm::ExtraObjectData::FLAGS_NEVER_FROZEN);
     return true;
 }