azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Detect concurrent modifications in ArrayDeque #KT-63355

Browse Source 
Remove the ArrayDeque.subList().subList() test for comodification detection.
For types that extend AbstractList and to not override subList function,
the behavior varies depending on JDK version. With JKD8, the test fails for ArrayDeque.
While with JDK9+, it passes.
This commit is contained in:
Abduqodiri Qurbonzoda
2024-01-12 18:46:10 +05:00
committed by Space Team
parent 5f16fb2e4d
commit c9d8ecc599
3 changed files with 113 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
libraries/stdlib/native-wasm/src/kotlin/collections/AbstractMutableList.kt
+10
View File
@@ -234,6 +234,16 @@ public actual abstract class AbstractMutableList<E> protected actual constructor
return _size
}
override fun iterator(): MutableIterator<E> {
checkForComodification()
return super.iterator()
}
override fun listIterator(index: Int): MutableListIterator<E> {
checkForComodification()
return super.listIterator(index)
}
private fun checkForComodification() {
if (list.modCount != modCount)
throw ConcurrentModificationException()
libraries/stdlib/src/kotlin/collections/ArrayDeque.kt
+21 -2
View File
@@ -120,6 +120,7 @@ public class ArrayDeque<E> : AbstractMutableList<E> {
* 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<E> : AbstractMutableList<E> {
* 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<E> : AbstractMutableList<E> {
*/
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<E> : AbstractMutableList<E> {
*/
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<E> : AbstractMutableList<E> {
return
}
registerModification()
ensureCapacity(size + 1)
// Elements in circular array lay in 2 ways:
@@ -269,6 +274,8 @@ public class ArrayDeque<E> : AbstractMutableList<E> {
public override fun addAll(elements: Collection<E>): 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<E> : AbstractMutableList<E> {
return addAll(elements)
}
registerModification()
ensureCapacity(this.size + elements.size)
val tail = internalIndex(size)
@@ -424,6 +432,8 @@ public class ArrayDeque<E> : AbstractMutableList<E> {
return removeFirst()
}
registerModification()
val internalIndex = internalIndex(index)
val element = internalGet(internalIndex)
@@ -510,14 +520,18 @@ public class ArrayDeque<E> : AbstractMutableList<E> {
}
}
}
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<E> : AbstractMutableList<E> {
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<E> : AbstractMutableList<E> {
}
}
registerModification()
if (fromIndex < size - toIndex) {
// closer to the first element -> shift preceding elements
removeRangeShiftPreceding(fromIndex, toIndex)
@@ -622,6 +637,10 @@ public class ArrayDeque<E> : AbstractMutableList<E> {
}
}
private fun registerModification() {
modCount += 1
}
// for testing
internal fun <T> testToArray(array: Array<T>): Array<T> = toArray(array)
internal fun testToArray(): Array<Any?> = toArray()
libraries/stdlib/test/collections/ConcurrentModificationTest.kt
+82 -7
View File
@@ -95,7 +95,12 @@ class ConcurrentModificationTest {
fun mutableList() {
if (TestPlatform.current == TestPlatform.Js) return
val operations = listOf<CollectionOperation<MutableList<String>>>(
/**
* 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<MutableList<String>>>(
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<MutableList<String>>) {
testIteratorThrowsCME(withMutableList, operations)
fun testThrowsCME(isJavaArrayListBehavior: Boolean = true, withMutableList: WithCollection<MutableList<String>>) {
testIteratorThrowsCME(withMutableList, operations(isJavaArrayListBehavior))
}
// size == capacity
@@ -146,6 +151,16 @@ class ConcurrentModificationTest {
}
}
testThrowsCME(isJavaArrayListBehavior = false) { action ->
ArrayDeque<String>(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<String>(10).apply {
@@ -160,6 +175,13 @@ class ConcurrentModificationTest {
action(this)
}
}
testThrowsCME(isJavaArrayListBehavior = false) { action ->
ArrayDeque<String>(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<ArrayDeque<String>>>(
CollectionOperation("addFirst()") { addFirst("e") },
CollectionOperation("addLast()") { addLast("e") },
CollectionOperation("removeFirst()") { removeFirst() },
CollectionOperation("removeLast()") { removeLast() },
CollectionOperation("removeFirstOrNull()") { removeFirstOrNull() },
CollectionOperation("removeLastOrNull()") { removeLastOrNull() },
)
fun testThrowsCME(withArrayDeque: WithCollection<ArrayDeque<String>>) {
testIteratorThrowsCME(withArrayDeque, operations)
}
// size == capacity
testThrowsCME { action ->
ArrayDeque<String>(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<String>(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<MutableList<String>>) {
@@ -244,6 +303,11 @@ class ConcurrentModificationTest {
arrayList.add("e")
action(subList)
}
assertFailsWith<ConcurrentModificationException> {
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<ConcurrentModificationException> {
val arrayDeque = ArrayDeque(listOf("a", "b", "c", "d"))
for (e in arrayDeque.subList(1, 3)) arrayDeque.remove(e)
}
}
@Test