diff --git a/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt b/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt index 2b41ce2fbcc..7e868192ffe 100644 --- a/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt +++ b/libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt @@ -234,6 +234,16 @@ public actual abstract class AbstractMutableList protected actual constructor return _size } + override fun iterator(): MutableIterator { + checkForComodification() + return super.iterator() + } + + override fun listIterator(index: Int): MutableListIterator { + checkForComodification() + return super.listIterator(index) + } + private fun checkForComodification() { if (list.modCount != modCount) throw ConcurrentModificationException() diff --git a/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt b/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt index 887e6f5ab27..fcb30b8fbae 100644 --- a/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt +++ b/libraries/stdlib/src/kotlin/collections/ArrayDeque.kt @@ -120,6 +120,7 @@ public class ArrayDeque : AbstractMutableList { * Prepends the specified [element] to this deque. */ public fun addFirst(element: E) { + registerModification() ensureCapacity(size + 1) head = decremented(head) @@ -131,6 +132,7 @@ public class ArrayDeque : AbstractMutableList { * Appends the specified [element] to this deque. */ public fun addLast(element: E) { + registerModification() ensureCapacity(size + 1) elementData[internalIndex(size)] = element @@ -142,6 +144,7 @@ public class ArrayDeque : AbstractMutableList { */ public fun removeFirst(): E { if (isEmpty()) throw NoSuchElementException("ArrayDeque is empty.") + registerModification() val element = internalGet(head) elementData[head] = null @@ -160,6 +163,7 @@ public class ArrayDeque : AbstractMutableList { */ public fun removeLast(): E { if (isEmpty()) throw NoSuchElementException("ArrayDeque is empty.") + registerModification() val internalLastIndex = internalIndex(lastIndex) val element = internalGet(internalLastIndex) @@ -190,6 +194,7 @@ public class ArrayDeque : AbstractMutableList { return } + registerModification() ensureCapacity(size + 1) // Elements in circular array lay in 2 ways: @@ -269,6 +274,8 @@ public class ArrayDeque : AbstractMutableList { public override fun addAll(elements: Collection): Boolean { if (elements.isEmpty()) return false + + registerModification() ensureCapacity(this.size + elements.size) copyCollectionElements(internalIndex(size), elements) return true @@ -283,6 +290,7 @@ public class ArrayDeque : AbstractMutableList { return addAll(elements) } + registerModification() ensureCapacity(this.size + elements.size) val tail = internalIndex(size) @@ -424,6 +432,8 @@ public class ArrayDeque : AbstractMutableList { return removeFirst() } + registerModification() + val internalIndex = internalIndex(index) val element = internalGet(internalIndex) @@ -510,14 +520,18 @@ public class ArrayDeque : AbstractMutableList { } } } - if (modified) + if (modified) { + registerModification() size = negativeMod(newTail - head) + } return modified } public override fun clear() { if (isNotEmpty()) { + registerModification() + val tail = internalIndex(size) nullifyNonEmpty(head, tail) } @@ -549,7 +563,6 @@ public class ArrayDeque : AbstractMutableList { override fun removeRange(fromIndex: Int, toIndex: Int) { AbstractList.checkRangeIndexes(fromIndex, toIndex, size) - // TODO: registerModification() val length = toIndex - fromIndex when (length) { @@ -564,6 +577,8 @@ public class ArrayDeque : AbstractMutableList { } } + registerModification() + if (fromIndex < size - toIndex) { // closer to the first element -> shift preceding elements removeRangeShiftPreceding(fromIndex, toIndex) @@ -622,6 +637,10 @@ public class ArrayDeque : AbstractMutableList { } } + private fun registerModification() { + modCount += 1 + } + // for testing internal fun testToArray(array: Array): Array = toArray(array) internal fun testToArray(): Array = toArray() diff --git a/libraries/stdlib/test/collections/ConcurrentModificationTest.kt b/libraries/stdlib/test/collections/ConcurrentModificationTest.kt index 0bc6eacab44..240947852a2 100644 --- a/libraries/stdlib/test/collections/ConcurrentModificationTest.kt +++ b/libraries/stdlib/test/collections/ConcurrentModificationTest.kt @@ -95,7 +95,12 @@ class ConcurrentModificationTest { fun mutableList() { if (TestPlatform.current == TestPlatform.Js) return - val operations = listOf>>( + /** + * Some operations should register a modification by contract, but java ArrayList, + * whose implementation we can't change, do not register. + * @param isJavaArrayListBehavior specifies whether to test java ArrayList behavior or the behavior by contract. + */ + fun operations(isJavaArrayListBehavior: Boolean) = listOf>>( CollectionOperation("set()", throwsCME = false) { set(2, "e") }, CollectionOperation("add()") { add("e") }, @@ -106,9 +111,9 @@ class ConcurrentModificationTest { CollectionOperation("removeAt()") { removeAt(2) }, CollectionOperation("addAll()") { addAll(listOf("e", "f")) }, - CollectionOperation("addAll(emptyList())") { addAll(emptyList()) }, + CollectionOperation("addAll(emptyList())", throwsCME = isJavaArrayListBehavior) { addAll(emptyList()) }, CollectionOperation("addAll(index)") { addAll(2, listOf("e", "f")) }, - CollectionOperation("addAll(index, emptyList())") { addAll(2, emptyList()) }, + CollectionOperation("addAll(index, emptyList())", throwsCME = isJavaArrayListBehavior) { addAll(2, emptyList()) }, CollectionOperation("removeAll(non-existing)", throwsCME = false) { removeAll(listOf("e", "f")) }, CollectionOperation("removeAll(some exist)") { removeAll(listOf("d", "e")) }, @@ -127,8 +132,8 @@ class ConcurrentModificationTest { } } - fun testThrowsCME(withMutableList: WithCollection>) { - testIteratorThrowsCME(withMutableList, operations) + fun testThrowsCME(isJavaArrayListBehavior: Boolean = true, withMutableList: WithCollection>) { + testIteratorThrowsCME(withMutableList, operations(isJavaArrayListBehavior)) } // size == capacity @@ -146,6 +151,16 @@ class ConcurrentModificationTest { } } + testThrowsCME(isJavaArrayListBehavior = false) { action -> + ArrayDeque(4).apply { + addAll(listOf("a", "b", "c", "d")) + action(this) + } + } + testThrowsCME(isJavaArrayListBehavior = false) { action -> + ArrayDeque(listOf("a", "b", "c", "d")).also(action) + } + // size < capacity testThrowsCME { action -> ArrayList(10).apply { @@ -160,6 +175,13 @@ class ConcurrentModificationTest { action(this) } } + + testThrowsCME(isJavaArrayListBehavior = false) { action -> + ArrayDeque(10).apply { + addAll(listOf("a", "b", "c", "d")) + action(this) + } + } } @Test @@ -195,6 +217,45 @@ class ConcurrentModificationTest { } } + @Test + fun arrayDeque() { + if (TestPlatform.current == TestPlatform.Js) return + + val operations = listOf>>( + CollectionOperation("addFirst()") { addFirst("e") }, + CollectionOperation("addLast()") { addLast("e") }, + + CollectionOperation("removeFirst()") { removeFirst() }, + CollectionOperation("removeLast()") { removeLast() }, + + CollectionOperation("removeFirstOrNull()") { removeFirstOrNull() }, + CollectionOperation("removeLastOrNull()") { removeLastOrNull() }, + ) + + fun testThrowsCME(withArrayDeque: WithCollection>) { + testIteratorThrowsCME(withArrayDeque, operations) + } + + // size == capacity + testThrowsCME { action -> + ArrayDeque(4).apply { + addAll(listOf("a", "b", "c", "d")) + action(this) + } + } + testThrowsCME { action -> + ArrayDeque(listOf("a", "b", "c", "d")).also(action) + } + + // size < capacity + testThrowsCME { action -> + ArrayDeque(10).apply { + addAll(listOf("a", "b", "c", "d")) + action(this) + } + } + } + @Test fun subList() { if (TestPlatform.current == TestPlatform.Js) return @@ -230,8 +291,6 @@ class ConcurrentModificationTest { CollectionOperation("clear()") { clear() }, CollectionOperation("iterator()") { iterator() }, CollectionOperation("listIterator()") { listIterator() }, - - CollectionOperation("subList()", throwsCME = false) { subList(0, 1) }, ) fun testThrowsCME(withMutableList: WithCollection>) { @@ -244,6 +303,11 @@ class ConcurrentModificationTest { arrayList.add("e") action(subList) } + assertFailsWith { + val arrayList = arrayListOf("a", "b", "c", "d") + for (e in arrayList.subList(1, 3)) arrayList.remove(e) + } + testThrowsCME { action -> buildList { addAll(listOf("a", "b", "c", "d")) @@ -258,6 +322,17 @@ class ConcurrentModificationTest { for (e in subList(1, 3)) remove(e) } } + + testThrowsCME { action -> + val arrayDeque = ArrayDeque(listOf("a", "b", "c", "d")) + val subList = arrayDeque.subList(0, arrayDeque.size) + arrayDeque.add("e") + action(subList) + } + assertFailsWith { + val arrayDeque = ArrayDeque(listOf("a", "b", "c", "d")) + for (e in arrayDeque.subList(1, 3)) arrayDeque.remove(e) + } } @Test