[JS IR] Catch concurrent modifications of HashMap

^KT-59001

libraries/stdlib/js/src/kotlin/collections/InternalHashMap.kt

@@ -6,15 +6,33 @@
 package kotlin.collections
 
 internal class InternalHashMap<K, V> private constructor(
+    // keys in insert order
     private var keysArray: Array<K>,
-    private var valuesArray: Array<V>?, // allocated only when actually used, always null in pure HashSet
+    // values in insert order, allocated only when actually used, always null in pure HashSet
+    private var valuesArray: Array<V>?,
+    // hash of a key by its index, -1 if a key at that index was removed
     private var presenceArray: IntArray,
+    // (index + 1) of a key by its hash, 0 if there is no key with that hash, -1 if collision chain continues to the hash-1
     private var hashArray: IntArray,
+    // max length of a collision chain
     private var maxProbeDistance: Int,
-    private var length: Int,
+    // index of the next key to be inserted
+    private var length: Int
 ) : InternalMap<K, V> {
     private var hashShift: Int = computeShift(hashSize)
 
+    /**
+     * The number of times this map is structurally modified.
+     *
+     * A modification is considered to be structural if it changes the map size,
+     * or otherwise changes it in a way that iterations in progress may return incorrect results.
+     *
+     * This value can be used by iterators of the [keys], [values] and [entries] views
+     * to provide fail-fast behavoir when a concurrent modification is detected during iteration.
+     * [ConcurrentModificationException] will be thrown in this case.
+     */
+    private var modCount: Int = 0
+
     private var _size: Int = 0
     override val size: Int
         get() = _size
@@ -133,6 +151,7 @@ internal class InternalHashMap<K, V> private constructor(
         valuesArray?.resetRange(0, length)
         _size = 0
         length = 0
+        registerModification()
     }
 
     override fun equals(other: Any?): Boolean {
@@ -169,6 +188,10 @@ internal class InternalHashMap<K, V> private constructor(
     private val capacity: Int get() = keysArray.size
     private val hashSize: Int get() = hashArray.size
 
+    private fun registerModification() {
+        modCount += 1
+    }
+
     override fun checkIsMutable() {
         if (isReadOnly) throw UnsupportedOperationException()
     }
@@ -231,6 +254,7 @@ internal class InternalHashMap<K, V> private constructor(
     }
 
     private fun rehash(newHashSize: Int) {
+        registerModification()
         if (length > _size) compact()
         if (newHashSize != hashSize) {
             hashArray = IntArray(newHashSize)
@@ -303,6 +327,7 @@ internal class InternalHashMap<K, V> private constructor(
                     presenceArray[putIndex] = hash
                     hashArray[hash] = putIndex + 1
                     _size++
+                    registerModification()
                     if (probeDistance > maxProbeDistance) maxProbeDistance = probeDistance
                     return putIndex
                 }
@@ -331,6 +356,7 @@ internal class InternalHashMap<K, V> private constructor(
         removeHashAt(presenceArray[index])
         presenceArray[index] = TOMBSTONE
         _size--
+        registerModification()
     }
 
     private fun removeHashAt(removedHash: Int) {
@@ -469,6 +495,7 @@ internal class InternalHashMap<K, V> private constructor(
     ) {
         internal var index = 0
         internal var lastIndex: Int = -1
+        private var expectedModCount: Int = map.modCount
 
         init {
             initNext()
@@ -482,14 +509,23 @@ internal class InternalHashMap<K, V> private constructor(
         fun hasNext(): Boolean = index < map.length
 
         fun remove() {
+            checkForComodification()
+            check(lastIndex != -1) { "Call next() before removing element from the iterator." }
             map.checkIsMutable()
             map.removeKeyAt(lastIndex)
             lastIndex = -1
+            expectedModCount = map.modCount
+        }
+
+        internal fun checkForComodification() {
+            if (map.modCount != expectedModCount)
+                throw ConcurrentModificationException()
         }
     }
 
     internal class KeysItr<K, V>(map: InternalHashMap<K, V>) : Itr<K, V>(map), MutableIterator<K> {
         override fun next(): K {
+            checkForComodification()
             if (index >= map.length) throw NoSuchElementException()
             lastIndex = index++
             val result = map.keysArray[lastIndex]
@@ -501,6 +537,7 @@ internal class InternalHashMap<K, V> private constructor(
 
     internal class ValuesItr<K, V>(map: InternalHashMap<K, V>) : Itr<K, V>(map), MutableIterator<V> {
         override fun next(): V {
+            checkForComodification()
             if (index >= map.length) throw NoSuchElementException()
             lastIndex = index++
             val result = map.valuesArray!![lastIndex]
@@ -511,6 +548,7 @@ internal class InternalHashMap<K, V> private constructor(
 
     internal class EntriesItr<K, V>(map: InternalHashMap<K, V>) : Itr<K, V>(map), MutableIterator<MutableMap.MutableEntry<K, V>> {
         override fun next(): EntryRef<K, V> {
+            checkForComodification()
             if (index >= map.length) throw NoSuchElementException()
             lastIndex = index++
             val result = EntryRef(map, lastIndex)

libraries/stdlib/js/src/kotlin/collections/InternalStringLinkedMap.kt

@@ -96,8 +96,10 @@ internal class InternalStringLinkedMap<K, V> : InternalStringMap<K, V>() {
     private abstract class BaseLinkedItr<K, V>(protected val map: InternalStringLinkedMap<K, V>) {
         protected var lastIndex = EMPTY_INDEX
         protected var index = map.headIndex
+        private var expectedModCount = map.modCount
 
         protected fun goNext() {
+            checkForComodification()
             if (index == EMPTY_INDEX) {
                 throw NoSuchElementException()
             }
@@ -108,11 +110,20 @@ internal class InternalStringLinkedMap<K, V> : InternalStringMap<K, V>() {
         fun hasNext(): Boolean = index != EMPTY_INDEX
 
         fun remove() {
+            checkForComodification()
+            check(lastIndex != EMPTY_INDEX) { "Call next() before removing element from the iterator." }
             map.removeKeyIndex(map.keys.getElement(lastIndex), lastIndex)
             if (index == map.size) {
                 index = lastIndex
             }
             lastIndex = EMPTY_INDEX
+            expectedModCount = map.modCount
+        }
+
+        private fun checkForComodification() {
+            if (map.modCount != expectedModCount) {
+                throw ConcurrentModificationException()
+            }
         }
     }
 

libraries/stdlib/js/src/kotlin/collections/InternalStringMap.kt

@@ -54,6 +54,22 @@ internal open class InternalStringMap<K, V> : InternalMap<K, V> {
     internal var values = createJsArray<V>()
     internal var keys = createJsArray<K>()
 
+    /**
+     * The number of times this map is structurally modified.
+     *
+     * A modification is considered to be structural if it changes the map size,
+     * or otherwise changes it in a way that iterations in progress may return incorrect results.
+     *
+     * This value can be used by iterators of the [keys], [values] and [entries] views
+     * to provide fail-fast behavoir when a concurrent modification is detected during iteration.
+     * [ConcurrentModificationException] will be thrown in this case.
+     */
+    internal var modCount: Int = 0
+
+    private fun registerModification() {
+        modCount += 1
+    }
+
     override val size: Int
         get() = keys.length
 
@@ -121,6 +137,7 @@ internal open class InternalStringMap<K, V> : InternalMap<K, V> {
         backingMap[key] = size
         keys.push(key)
         values.push(value)
+        registerModification()
         return null
     }
 
@@ -155,12 +172,14 @@ internal open class InternalStringMap<K, V> : InternalMap<K, V> {
             values.replaceElementAtWithLast(removingIndex)
             backingMap[keys.getElement(removingIndex)] = removingIndex
         }
+        registerModification()
     }
 
     override fun clear() {
         backingMap = createJsMap()
         keys = createJsArray()
         values = createJsArray()
+        registerModification()
     }
 
     override fun build() {
@@ -177,8 +196,10 @@ internal open class InternalStringMap<K, V> : InternalMap<K, V> {
     private abstract class BaseItr<K, V>(protected val map: InternalStringMap<K, V>) {
         protected var lastIndex = -1
         protected var index = 0
+        private var expectedModCount = map.modCount
 
         protected fun goNext() {
+            checkForComodification()
             if (index >= map.size) {
                 throw NoSuchElementException()
             }
@@ -188,9 +209,18 @@ internal open class InternalStringMap<K, V> : InternalMap<K, V> {
         fun hasNext(): Boolean = index < map.size
 
         fun remove() {
+            checkForComodification()
+            check(lastIndex != -1) { "Call next() before removing element from the iterator." }
             map.removeKeyIndex(map.keys.getElement(lastIndex), lastIndex)
             index = lastIndex
             lastIndex = -1
+            expectedModCount = map.modCount
+        }
+
+        private fun checkForComodification() {
+            if (map.modCount != expectedModCount) {
+                throw ConcurrentModificationException()
+            }
         }
     }
 

libraries/stdlib/test/collections/ConcurrentModificationTest.kt

@@ -6,6 +6,10 @@
 package test.collections
 
 import test.TestPlatform
+import test.collections.js.linkedStringMapOf
+import test.collections.js.linkedStringSetOf
+import test.collections.js.stringMapOf
+import test.collections.js.stringSetOf
 import test.current
 import kotlin.test.Test
 import kotlin.test.assertFailsWith
@@ -167,8 +171,6 @@ class ConcurrentModificationTest {
 
     @Test
     fun mutableSet() {
-        if (TestPlatform.current == TestPlatform.Js) return
-
         val operations = listOf<CollectionOperation<MutableSet<String>>>(
             CollectionOperation("add(non-existing)") { add("e") },
             CollectionOperation("add(existing)", throwsCME = false) { add("d") },
@@ -226,12 +228,25 @@ class ConcurrentModificationTest {
                 action(this)
             }
         }
+
+        if (TestPlatform.current == TestPlatform.Js) {
+            testThrowsCME { action ->
+                stringSetOf().apply {
+                    addAll(elements)
+                    action(this)
+                }
+            }
+            testThrowsCME { action ->
+                linkedStringSetOf().apply {
+                    addAll(elements)
+                    action(this)
+                }
+            }
+        }
     }
 
     @Test
     fun mutableMap() {
-        if (TestPlatform.current == TestPlatform.Js) return
-
         val operations = listOf<CollectionOperation<MutableMap<String, String>>>(
             CollectionOperation("put(non-existing)") { put("e", "e") },
             CollectionOperation("put(existing)", throwsCME = false) { put("d", "d") },
@@ -285,6 +300,21 @@ class ConcurrentModificationTest {
                 action(this)
             }
         }
+
+        if (TestPlatform.current == TestPlatform.Js) {
+            testThrowsCME { action ->
+                stringMapOf<String>().apply {
+                    putAll(entries)
+                    action(this)
+                }
+            }
+            testThrowsCME { action ->
+                linkedStringMapOf<String>().apply {
+                    putAll(entries)
+                    action(this)
+                }
+            }
+        }
     }
 }
 
@@ -311,4 +341,4 @@ private val listIteratorOperations = listOf<IteratorOperation<MutableListIterato
     IteratorOperation("remove()", { next() }) { remove() },
     IteratorOperation("previous()", { next() }) { previous() },
     IteratorOperation("add(\"e\")") { add("e") }
-)
+)