Detect concurrent modification of the parent list in subList

As a part of efforts to stabilize Native stdlib.

libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt

@@ -21,6 +21,10 @@ internal class ListBuilder<E> private constructor(
         private val Empty = ListBuilder<Nothing>(0).also { it.isReadOnly = true }
     }
 
+    init {
+        if (backing != null) this.modCount = backing.modCount
+    }
+
     constructor() : this(10)
 
     constructor(initialCapacity: Int) : this(
@@ -40,17 +44,25 @@ internal class ListBuilder<E> private constructor(
             throw NotSerializableException("The list cannot be serialized while it is being built.")
 
     override val size: Int
-        get() = length
+        get() {
+            checkForComodification()
+            return length
+        }
 
-    override fun isEmpty(): Boolean = length == 0
+    override fun isEmpty(): Boolean {
+        checkForComodification()
+        return length == 0
+    }
 
     override fun get(index: Int): E {
+        checkForComodification()
         AbstractList.checkElementIndex(index, length)
         return array[offset + index]
     }
 
     override operator fun set(index: Int, element: E): E {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkElementIndex(index, length)
         val old = array[offset + index]
         array[offset + index] = element
@@ -58,6 +70,7 @@ internal class ListBuilder<E> private constructor(
     }
 
     override fun indexOf(element: E): Int {
+        checkForComodification()
         var i = 0
         while (i < length) {
             if (array[offset + i] == element) return i
@@ -67,6 +80,7 @@ internal class ListBuilder<E> private constructor(
     }
 
     override fun lastIndexOf(element: E): Int {
+        checkForComodification()
         var i = length - 1
         while (i >= 0) {
             if (array[offset + i] == element) return i
@@ -75,28 +89,32 @@ internal class ListBuilder<E> private constructor(
         return -1
     }
 
-    override fun iterator(): MutableIterator<E> = Itr(this, 0)
+    override fun iterator(): MutableIterator<E> = listIterator(0)
-    override fun listIterator(): MutableListIterator<E> = Itr(this, 0)
+    override fun listIterator(): MutableListIterator<E> = listIterator(0)
 
     override fun listIterator(index: Int): MutableListIterator<E> {
+        checkForComodification()
         AbstractList.checkPositionIndex(index, length)
         return Itr(this, index)
     }
 
     override fun add(element: E): Boolean {
         checkIsMutable()
+        checkForComodification()
         addAtInternal(offset + length, element)
         return true
     }
 
     override fun add(index: Int, element: E) {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkPositionIndex(index, length)
         addAtInternal(offset + index, element)
     }
 
     override fun addAll(elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         val n = elements.size
         addAllInternal(offset + length, elements, n)
         return n > 0
@@ -104,6 +122,7 @@ internal class ListBuilder<E> private constructor(
 
     override fun addAll(index: Int, elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkPositionIndex(index, length)
         val n = elements.size
         addAllInternal(offset + index, elements, n)
@@ -112,17 +131,20 @@ internal class ListBuilder<E> private constructor(
 
     override fun clear() {
         checkIsMutable()
+        checkForComodification()
         removeRangeInternal(offset, length)
     }
 
     override fun removeAt(index: Int): E {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkElementIndex(index, length)
         return removeAtInternal(offset + index)
     }
 
     override fun remove(element: E): Boolean {
         checkIsMutable()
+        checkForComodification()
         val i = indexOf(element)
         if (i >= 0) removeAt(i)
         return i >= 0
@@ -130,11 +152,13 @@ internal class ListBuilder<E> private constructor(
 
     override fun removeAll(elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         return retainOrRemoveAllInternal(offset, length, elements, false) > 0
     }
 
     override fun retainAll(elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         return retainOrRemoveAllInternal(offset, length, elements, true) > 0
     }
 
@@ -144,6 +168,7 @@ internal class ListBuilder<E> private constructor(
     }
 
     override fun <T> toArray(destination: Array<T>): Array<T> {
+        checkForComodification()
         if (destination.size < length) {
             return java.util.Arrays.copyOfRange(array, offset, offset + length, destination.javaClass)
         }
@@ -160,20 +185,24 @@ internal class ListBuilder<E> private constructor(
     }
 
     override fun toArray(): Array<Any?> {
+        checkForComodification()
         @Suppress("UNCHECKED_CAST")
         return array.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array<Any?>
     }
 
     override fun equals(other: Any?): Boolean {
+        checkForComodification()
         return other === this ||
                 (other is List<*>) && contentEquals(other)
     }
 
     override fun hashCode(): Int {
+        checkForComodification()
         return array.subarrayContentHashCode(offset, length)
     }
 
     override fun toString(): String {
+        checkForComodification()
         return array.subarrayContentToString(offset, length, this)
     }
 
@@ -183,6 +212,11 @@ internal class ListBuilder<E> private constructor(
         modCount += 1
     }
 
+    private fun checkForComodification() {
+        if (root != null && root.modCount != modCount)
+            throw ConcurrentModificationException()
+    }
+
     private fun checkIsMutable() {
         if (isEffectivelyReadOnly) throw UnsupportedOperationException()
     }

libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt

@@ -185,9 +185,11 @@ public actual abstract class AbstractMutableList<E> protected actual constructor
         init {
             AbstractList.checkRangeIndexes(fromIndex, toIndex, list.size)
             this._size = toIndex - fromIndex
+            this.modCount = list.modCount
         }
 
         override fun add(index: Int, element: E) {
+            checkForComodification()
             AbstractList.checkPositionIndex(index, _size)
 
             list.add(fromIndex + index, element)
@@ -196,12 +198,14 @@ public actual abstract class AbstractMutableList<E> protected actual constructor
         }
 
         override fun get(index: Int): E {
+            checkForComodification()
             AbstractList.checkElementIndex(index, _size)
 
             return list[fromIndex + index]
         }
 
         override fun removeAt(index: Int): E {
+            checkForComodification()
             AbstractList.checkElementIndex(index, _size)
 
             val result = list.removeAt(fromIndex + index)
@@ -211,12 +215,22 @@ public actual abstract class AbstractMutableList<E> protected actual constructor
         }
 
         override fun set(index: Int, element: E): E {
+            checkForComodification()
             AbstractList.checkElementIndex(index, _size)
 
             return list.set(fromIndex + index, element)
         }
 
-        override val size: Int get() = _size
+        override val size: Int
+            get() {
+                checkForComodification()
+                return _size
+            }
+
+        private fun checkForComodification() {
+            if (list.modCount != modCount)
+                throw ConcurrentModificationException()
+        }
     }
 
 }

libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt

@@ -17,6 +17,10 @@ actual class ArrayList<E> private constructor(
         private val Empty = ArrayList<Nothing>(0).also { it.isReadOnly = true }
     }
 
+    init {
+        if (backingList != null) this.modCount = backingList.modCount
+    }
+
     /**
      * Creates a new empty [ArrayList].
      */
@@ -55,17 +59,25 @@ actual class ArrayList<E> private constructor(
     }
 
     override actual val size: Int
-        get() = length
+        get() {
+            checkForComodification()
+            return length
+        }
 
-    override actual fun isEmpty(): Boolean = length == 0
+    override actual fun isEmpty(): Boolean {
+        checkForComodification()
+        return length == 0
+    }
 
     override actual fun get(index: Int): E {
+        checkForComodification()
         AbstractList.checkElementIndex(index, length)
         return backingArray[offset + index]
     }
 
     override actual operator fun set(index: Int, element: E): E {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkElementIndex(index, length)
         val old = backingArray[offset + index]
         backingArray[offset + index] = element
@@ -73,6 +85,7 @@ actual class ArrayList<E> private constructor(
     }
 
     override actual fun indexOf(element: E): Int {
+        checkForComodification()
         var i = 0
         while (i < length) {
             if (backingArray[offset + i] == element) return i
@@ -82,6 +95,7 @@ actual class ArrayList<E> private constructor(
     }
 
     override actual fun lastIndexOf(element: E): Int {
+        checkForComodification()
         var i = length - 1
         while (i >= 0) {
             if (backingArray[offset + i] == element) return i
@@ -90,28 +104,32 @@ actual class ArrayList<E> private constructor(
         return -1
     }
 
-    override actual fun iterator(): MutableIterator<E> = Itr(this, 0)
+    override actual fun iterator(): MutableIterator<E> = listIterator(0)
-    override actual fun listIterator(): MutableListIterator<E> = Itr(this, 0)
+    override actual fun listIterator(): MutableListIterator<E> = listIterator(0)
 
     override actual fun listIterator(index: Int): MutableListIterator<E> {
+        checkForComodification()
         AbstractList.checkPositionIndex(index, length)
         return Itr(this, index)
     }
 
     override actual fun add(element: E): Boolean {
         checkIsMutable()
+        checkForComodification()
         addAtInternal(offset + length, element)
         return true
     }
 
     override actual fun add(index: Int, element: E) {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkPositionIndex(index, length)
         addAtInternal(offset + index, element)
     }
 
     override actual fun addAll(elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         val n = elements.size
         addAllInternal(offset + length, elements, n)
         return n > 0
@@ -119,6 +137,7 @@ actual class ArrayList<E> private constructor(
 
     override actual fun addAll(index: Int, elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkPositionIndex(index, length)
         val n = elements.size
         addAllInternal(offset + index, elements, n)
@@ -127,17 +146,20 @@ actual class ArrayList<E> private constructor(
 
     override actual fun clear() {
         checkIsMutable()
+        checkForComodification()
         removeRangeInternal(offset, length)
     }
 
     override actual fun removeAt(index: Int): E {
         checkIsMutable()
+        checkForComodification()
         AbstractList.checkElementIndex(index, length)
         return removeAtInternal(offset + index)
     }
 
     override actual fun remove(element: E): Boolean {
         checkIsMutable()
+        checkForComodification()
         val i = indexOf(element)
         if (i >= 0) removeAt(i)
         return i >= 0
@@ -145,11 +167,13 @@ actual class ArrayList<E> private constructor(
 
     override actual fun removeAll(elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         return retainOrRemoveAllInternal(offset, length, elements, false) > 0
     }
 
     override actual fun retainAll(elements: Collection<E>): Boolean {
         checkIsMutable()
+        checkForComodification()
         return retainOrRemoveAllInternal(offset, length, elements, true) > 0
     }
 
@@ -173,20 +197,24 @@ actual class ArrayList<E> private constructor(
     }
 
     override fun equals(other: Any?): Boolean {
+        checkForComodification()
         return other === this ||
                 (other is List<*>) && contentEquals(other)
     }
 
     override fun hashCode(): Int {
+        checkForComodification()
         return backingArray.subarrayContentHashCode(offset, length)
     }
 
     override fun toString(): String {
+        checkForComodification()
         return backingArray.subarrayContentToString(offset, length, this)
     }
 
     @Suppress("UNCHECKED_CAST")
     override fun <T> toArray(array: Array<T>): Array<T> {
+        checkForComodification()
         if (array.size < length) {
             return backingArray.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array<T>
         }
@@ -201,6 +229,7 @@ actual class ArrayList<E> private constructor(
     }
 
     override fun toArray(): Array<Any?> {
+        checkForComodification()
         @Suppress("UNCHECKED_CAST")
         return backingArray.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array<Any?>
     }
@@ -211,6 +240,11 @@ actual class ArrayList<E> private constructor(
         modCount += 1
     }
 
+    private fun checkForComodification() {
+        if (root != null && root.modCount != modCount)
+            throw ConcurrentModificationException()
+    }
+
     private fun checkIsMutable() {
         if (isReadOnly || root != null && root.isReadOnly) throw UnsupportedOperationException()
     }

libraries/stdlib/test/collections/ConcurrentModificationTest.kt

@@ -65,6 +65,32 @@ class ConcurrentModificationTest {
         }
     }
 
+    private fun <C : MutableList<String>> testSubListThrowsCME(
+        withSubList: WithCollection<C>,
+        subListOps: List<CollectionOperation<C>>
+    ) {
+        var invoked = false
+        withSubList { subList ->
+            invoked = true
+
+            for (subListOp in subListOps) {
+                val message = "subListOp: ${subListOp.description}"
+                if (subListOp.throwsCME) {
+                    assertFailsWith<ConcurrentModificationException>(message) {
+                        subListOp.function.invoke(subList)
+                    }
+                } else {
+                    try {
+                        subListOp.function.invoke(subList)
+                    } catch (e: Throwable) {
+                        fail("$message. Expected no exception, but was $e")
+                    }
+                }
+            }
+        }
+        assertTrue(invoked)
+    }
+
     @Test
     fun mutableList() {
         if (TestPlatform.current == TestPlatform.Js) return
@@ -169,6 +195,65 @@ class ConcurrentModificationTest {
         }
     }
 
+    @Test
+    fun subList() {
+        if (TestPlatform.current == TestPlatform.Js) return
+
+        val operations = listOf<CollectionOperation<MutableList<String>>>(
+            CollectionOperation("isEmpty()") { isEmpty() },
+            CollectionOperation("size") { size },
+
+            CollectionOperation("equals()") { equals(listOf("x")) },
+            CollectionOperation("hashCode()") { hashCode() },
+            CollectionOperation("toString()") { toString() },
+
+            CollectionOperation("indexOf") { indexOf("d") },
+            CollectionOperation("lastIndexOf") { lastIndexOf("d") },
+            CollectionOperation("contains") { contains("d") },
+
+            CollectionOperation("get()") { get(2) },
+            CollectionOperation("set()") { set(2, "e") },
+
+            CollectionOperation("add()") { add("e") },
+            CollectionOperation("add(index)") { add(2, "e") },
+
+            CollectionOperation("remove()") { remove("d") },
+            CollectionOperation("removeAt()") { removeAt(2) },
+
+            CollectionOperation("addAll()") { addAll(listOf("e", "f")) },
+            CollectionOperation("addAll(index)") { addAll(2, listOf("e", "f")) },
+
+            CollectionOperation("removeAll()") { removeAll(listOf("d", "e")) },
+
+            CollectionOperation("retainAll()") { retainAll(listOf("d", "e")) },
+
+            CollectionOperation("clear()") { clear() },
+            CollectionOperation("iterator()") { iterator() },
+            CollectionOperation("listIterator()") { listIterator() },
+
+            CollectionOperation("subList()", throwsCME = false) { subList(0, 1) },
+        )
+
+        fun testThrowsCME(withMutableList: WithCollection<MutableList<String>>) {
+            testSubListThrowsCME(withMutableList, operations)
+        }
+
+        testThrowsCME { action ->
+            val arrayList = arrayListOf("a", "b", "c", "d")
+            val subList = arrayList.subList(0, arrayList.size)
+            arrayList.add("e")
+            action(subList)
+        }
+        testThrowsCME { action ->
+            buildList {
+                addAll(listOf("a", "b", "c", "d"))
+                val subList = subList(0, size)
+                add("e")
+                action(subList)
+            }
+        }
+    }
+
     @Test
     fun mutableSet() {
         val operations = listOf<CollectionOperation<MutableSet<String>>>(