Rework lazy due to cyclic collector. (#3861)

backend.native/tests/runtime/workers/lazy1.kt

@@ -6,17 +6,55 @@
 package runtime.workers.lazy1
 
 import kotlin.test.*
-
 import kotlin.native.concurrent.*
 
-class Leak {
+class Lazy {
-    val leak by lazy { this }
+    val x = 17
+    val self by lazy { this }
+    val recursion: Int by lazy {
+        if (x < 17) 42 else recursion
+    }
+    val freezer: Int by lazy {
+        freeze()
+        42
+    }
+    val thrower: String by lazy {
+        if (x < 100) throw IllegalArgumentException()
+        "FAIL"
+    }
 }
 
-@Test fun runTest() {
+@Test fun runTest1() {
-    assertFailsWith<InvalidMutabilityException> {
+    assertFailsWith<IllegalStateException> {
-        for (i in 1..100)
+        println(Lazy().recursion)
-            Leak().freeze().leak
+    }
+    assertFailsWith<IllegalStateException> {
+        println(Lazy().freeze().recursion)
+    }
+}
+
+@Test fun runTest2() {
+    var sum = 0
+    for (i in 1 .. 100) {
+        val self = Lazy().freeze()
+        assertEquals(self, self.self)
+        sum += self.self.hashCode()
     }
     println("OK")
 }
+
+
+@Test fun runTest3() {
+    assertFailsWith<InvalidMutabilityException> {
+        println(Lazy().freezer)
+    }
+}
+
+@Test fun runTest4() {
+    val self = Lazy()
+    repeat(10) {
+        assertFailsWith<IllegalArgumentException> {
+            println(self.thrower)
+        }
+    }
+}

runtime/src/main/cpp/Memory.cpp

@@ -2545,49 +2545,6 @@ void ensureNeverFrozen(ObjHeader* object) {
    object->meta_object()->flags_ |= MF_NEVER_FROZEN;
 }
 
-// TODO: incorrect, use 3-color scheme.
-KBoolean ensureAcyclicAndSet(ObjHeader* where, KInt index, ObjHeader* what) {
-    RuntimeAssert(where->container() != nullptr && where->container()->frozen(), "Must be used on frozen objects only");
-    RuntimeAssert(what == nullptr || isPermanentOrFrozen(what),
-        "Must be used with an immutable value");
-    if (what != nullptr) {
-        // Now we check that `where` is not reachable from `what`.
-        // As we cannot modify objects while traversing, instead we remember all seen objects in a set.
-        KStdUnorderedSet<ContainerHeader*> seen;
-        KStdDeque<ContainerHeader*> queue;
-        if (what->container() != nullptr)
-            queue.push_back(what->container());
-        bool acyclic = true;
-        while (!queue.empty() && acyclic) {
-            ContainerHeader* current = queue.front();
-            queue.pop_front();
-            seen.insert(current);
-            if (isAggregatingFrozenContainer(current)) {
-                ContainerHeader** subContainer = reinterpret_cast<ContainerHeader**>(current + 1);
-                for (int i = 0; i < current->objectCount(); ++i) {
-                    if (seen.count(*subContainer) == 0)
-                        queue.push_back(*subContainer++);
-                }
-            } else {
-              traverseContainerReferredObjects(current, [where, &queue, &acyclic, &seen](ObjHeader* obj) {
-                if (obj == where) {
-                    acyclic = false;
-                } else {
-                    auto* objContainer = obj->container();
-                    if (objContainer != nullptr && seen.count(objContainer) == 0)
-                        queue.push_back(objContainer);
-                }
-              });
-            }
-          }
-        if (!acyclic) return false;
-    }
-    UpdateHeapRef(reinterpret_cast<ObjHeader**>(
-            reinterpret_cast<uintptr_t>(where) + where->type_info()->objOffsets_[index]), what);
-    // Fence on updated location?
-    return true;
-}
-
 void shareAny(ObjHeader* obj) {
   auto* container = obj->container();
   if (isShareable(container)) return;
@@ -3040,10 +2997,6 @@ void EnsureNeverFrozen(ObjHeader* object) {
   ensureNeverFrozen(object);
 }
 
-KBoolean Konan_ensureAcyclicAndSet(ObjHeader* where, KInt index, ObjHeader* what) {
-  return ensureAcyclicAndSet(where, index, what);
-}
-
 void Kotlin_Any_share(ObjHeader* obj) {
   shareAny(obj);
 }

runtime/src/main/kotlin/kotlin/Lazy.kt

@@ -34,7 +34,7 @@ public actual fun <T> lazy(initializer: () -> T): Lazy<T> = FreezeAwareLazyImpl(
 public actual fun <T> lazy(mode: LazyThreadSafetyMode, initializer: () -> T): Lazy<T> =
         when (mode) {
             LazyThreadSafetyMode.SYNCHRONIZED -> throw UnsupportedOperationException()
-            LazyThreadSafetyMode.PUBLICATION -> throw UnsupportedOperationException()
+            LazyThreadSafetyMode.PUBLICATION -> FreezeAwareLazyImpl(initializer)
             LazyThreadSafetyMode.NONE -> UnsafeLazyImpl(initializer)
         }
 

runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt

@@ -6,67 +6,61 @@
 package kotlin.native.concurrent
 
 import kotlin.native.internal.Frozen
-import kotlin.native.internal.NoReorderFields
 
-@SymbolName("Konan_ensureAcyclicAndSet")
-private external fun ensureAcyclicAndSet(where: Any, index: Int, what: Any?): Boolean
-
-@SymbolName("ReadHeapRefNoLock")
-internal external fun readHeapRefNoLock(where: Any, index: Int): Any?
-
-@NoReorderFields
 internal class FreezeAwareLazyImpl<out T>(initializer: () -> T) : Lazy<T> {
-    // IMPORTANT: due to simplified ensureAcyclicAndSet() semantics fields here must be ordered like this,
+    private val value_ = FreezableAtomicReference<Any?>(UNINITIALIZED)
-    // as an ordinal is used to refer a field.
+    private val initializer_ = FreezableAtomicReference<(() -> T)?>(initializer)
-    private var value_: Any? = UNINITIALIZED
-    private var initializer_: (() -> T)? = initializer
     private val lock_ = Lock()
 
+    private fun getOrInit(doFreeze: Boolean): T {
+        var result = value_.value
+        if (result !== UNINITIALIZED) {
+            if (result === INITIALIZING) {
+                value_.value = UNINITIALIZED
+                throw IllegalStateException("Recursive lazy computation")
+            }
+            @Suppress("UNCHECKED_CAST")
+            return result as T
+        }
+        // Set value_ to INITIALIZING.
+        value_.value = INITIALIZING
+        try {
+            result = initializer_.value!!()
+            if (doFreeze) result.freeze()
+        } catch (throwable: Throwable) {
+            value_.value = UNINITIALIZED
+            throw throwable
+        }
+        if (!doFreeze && this.isFrozen) {
+            value_.value = UNINITIALIZED
+            throw InvalidMutabilityException("Frozen during lazy computation")
+        }
+        // Set value_ to actual one.
+        value_.value = result
+        // Clear initializer.
+        initializer_.value = null
+        @Suppress("UNCHECKED_CAST")
+        return result as T
+    }
+
     override val value: T
         get() {
-            if (isFrozen) {
+            return if (isFrozen) {
                 locked(lock_) {
-                    // Lock is already taken above.
+                    getOrInit(true)
-                    var result = readHeapRefNoLock(this, 0)
-                    if (result !== UNINITIALIZED) {
-                        assert(result !== INITIALIZING)
-                        @Suppress("UNCHECKED_CAST")
-                        return result as T
-                    }
-                    // Set value_ to INITIALIZING.
-                    ensureAcyclicAndSet(this, 0, INITIALIZING)
-                    result = initializer_!!().freeze()
-                    // Set value_.
-                    if (!ensureAcyclicAndSet(this, 0, result)) {
-                        throw InvalidMutabilityException("Setting cyclic data via lazy in $this: $result")
-                    }
-                    // Do not clear initializer_ reference, as it may break freezing invariants and zero out
-                    // still valid object. It seems to be safe only in case when `this` is not reachable from
-                    // initializer.
-                    @Suppress("UNCHECKED_CAST")
-                    return result as T
                 }
             } else {
-                var result: Any? = value_
+                getOrInit(false)
-                if (result === UNINITIALIZED) {
-                    result = initializer_!!()
-                    if (isFrozen)
-                        throw InvalidMutabilityException("$this got frozen during lazy evaluation" )
-                    value_ = result
-                    initializer_ = null
-                }
-                @Suppress("UNCHECKED_CAST")
-                return result as T
             }
         }
 
     /**
      * This operation on shared objects may return value which is no longer reflect the current state of lazy.
      */
-    override fun isInitialized(): Boolean = (value_ !== UNINITIALIZED) && (value_ !== INITIALIZING)
+    override fun isInitialized(): Boolean = (value_.value !== UNINITIALIZED) && (value_.value !== INITIALIZING)
 
     override fun toString(): String = if (isInitialized())
-        value.toString() else "Lazy value not initialized yet."
+        value.toString() else "Lazy value not initialized yet"
 }
 
 internal object UNINITIALIZED {

runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt

@@ -14,6 +14,9 @@ external private fun Any.share()
 @SymbolName("Kotlin_CPointer_CopyMemory")
 external private fun CopyMemory(to: COpaquePointer?, from: COpaquePointer?, count: Int)
 
+@SymbolName("ReadHeapRefNoLock")
+internal external fun readHeapRefNoLock(where: Any, index: Int): Any?
+
 /**
  * Mutable concurrently accessible data buffer. Could be accessed from several workers simulteniously.
  */