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

[K/N] Catch concurrent modifications of HashMap

Browse Source 
As a part of efforts to stabilize Native stdlib.
This commit is contained in:
Abduqodiri Qurbonzoda
2023-05-25 03:59:49 +03:00
committed by Space Team
parent 9d8d9d000f
commit af1630e270
3 changed files with 197 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
libraries/stdlib/jvm/src/kotlin/collections/builders/MapBuilder.kt
+31
View File
@@ -25,6 +25,18 @@ internal class MapBuilder<K, V> private constructor(
) : MutableMap<K, V>, Serializable {
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
override var size: Int = 0
private set
@@ -112,6 +124,7 @@ internal class MapBuilder<K, V> private constructor(
valuesArray?.resetRange(0, length)
size = 0
length = 0
registerModification()
}
override val keys: MutableSet<K> get() {
@@ -176,6 +189,10 @@ internal class MapBuilder<K, V> private constructor(
internal val capacity: Int get() = keysArray.size
private val hashSize: Int get() = hashArray.size
private fun registerModification() {
modCount += 1
}
internal fun checkIsMutable() {
if (isReadOnly) throw UnsupportedOperationException()
}
@@ -237,6 +254,7 @@ internal class MapBuilder<K, V> private constructor(
}
private fun rehash(newHashSize: Int) {
registerModification()
if (length > size) compact()
if (newHashSize != hashSize) {
hashArray = IntArray(newHashSize)
@@ -308,6 +326,7 @@ internal class MapBuilder<K, V> private constructor(
presenceArray[putIndex] = hash
hashArray[hash] = putIndex + 1
size++
registerModification()
if (probeDistance > maxProbeDistance) maxProbeDistance = probeDistance
return putIndex
}
@@ -336,6 +355,7 @@ internal class MapBuilder<K, V> private constructor(
removeHashAt(presenceArray[index])
presenceArray[index] = TOMBSTONE
size--
registerModification()
}
private fun removeHashAt(removedHash: Int) {
@@ -477,6 +497,7 @@ internal class MapBuilder<K, V> private constructor(
) {
internal var index = 0
internal var lastIndex: Int = -1
private var expectedModCount: Int = map.modCount
init {
initNext()
@@ -490,15 +511,23 @@ internal class MapBuilder<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: MapBuilder<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]
@@ -510,6 +539,7 @@ internal class MapBuilder<K, V> private constructor(
internal class ValuesItr<K, V>(map: MapBuilder<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]
@@ -521,6 +551,7 @@ internal class MapBuilder<K, V> private constructor(
internal class EntriesItr<K, V>(map: MapBuilder<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/native-wasm/src/kotlin/collections/HashMap.kt
+44 -6
View File
@@ -10,15 +10,33 @@ import kotlin.native.FreezingIsDeprecated
@OptIn(FreezingIsDeprecated::class)
actual class HashMap<K, V> private constructor(
private var keysArray: Array<K>,
private var valuesArray: Array<V>?, // allocated only when actually used, always null in pure HashSet
private var presenceArray: IntArray,
private var hashArray: IntArray,
private var maxProbeDistance: Int,
private var length: Int
// keys in insert order
private var keysArray: Array<K>,
// 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,
// index of the next key to be inserted
private var length: Int
) : MutableMap<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 actual val size: Int
get() = _size
@@ -140,6 +158,7 @@ actual class HashMap<K, V> private constructor(
valuesArray?.resetRange(0, length)
_size = 0
length = 0
registerModification()
}
override actual val keys: MutableSet<K> get() {
@@ -206,6 +225,10 @@ actual class HashMap<K, V> private constructor(
private val capacity: Int get() = keysArray.size
private val hashSize: Int get() = hashArray.size
private fun registerModification() {
modCount += 1
}
internal fun checkIsMutable() {
if (isReadOnly) throw UnsupportedOperationException()
}
@@ -268,6 +291,7 @@ actual class HashMap<K, V> private constructor(
}
private fun rehash(newHashSize: Int) {
registerModification()
if (length > _size) compact()
if (newHashSize != hashSize) {
hashArray = IntArray(newHashSize)
@@ -339,6 +363,7 @@ actual class HashMap<K, V> private constructor(
presenceArray[putIndex] = hash
hashArray[hash] = putIndex + 1
_size++
registerModification()
if (probeDistance > maxProbeDistance) maxProbeDistance = probeDistance
return putIndex
}
@@ -367,6 +392,7 @@ actual class HashMap<K, V> private constructor(
removeHashAt(presenceArray[index])
presenceArray[index] = TOMBSTONE
_size--
registerModification()
}
private fun removeHashAt(removedHash: Int) {
@@ -534,6 +560,7 @@ actual class HashMap<K, V> private constructor(
) {
internal var index = 0
internal var lastIndex: Int = -1
private var expectedModCount: Int = map.modCount
init {
initNext()
@@ -547,14 +574,23 @@ actual class HashMap<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: HashMap<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]
@@ -566,6 +602,7 @@ actual class HashMap<K, V> private constructor(
internal class ValuesItr<K, V>(map: HashMap<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]
@@ -577,6 +614,7 @@ actual class HashMap<K, V> private constructor(
internal class EntriesItr<K, V>(map: HashMap<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/test/collections/ConcurrentModificationTest.kt
+122
View File
@@ -166,6 +166,128 @@ 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") },
CollectionOperation("remove(non-existing)", throwsCME = false) { remove("e") },
CollectionOperation("remove(existing)") { remove("d") },
CollectionOperation("addAll(emptyList())", throwsCME = false) { addAll(emptyList()) },
CollectionOperation("addAll(existing)", throwsCME = false) { addAll(listOf("d", "b")) },
CollectionOperation("addAll(some exist)") { addAll(listOf("d", "e")) },
CollectionOperation("addAll(non-existing)") { addAll(listOf("e", "f")) },
CollectionOperation("removeAll(non-existing)", throwsCME = false) { removeAll(listOf("e", "f")) },
CollectionOperation("removeAll(some exist") { removeAll(listOf("d", "e")) },
CollectionOperation("removeAll(emptyList())", throwsCME = false) { removeAll(emptyList()) },
CollectionOperation("retainAll(this.toMutableSet())", throwsCME = false) { retainAll(this.toMutableSet()) },
CollectionOperation("retainAll(non-existing)") { retainAll(listOf("e", "f")) },
CollectionOperation("retainAll(some exist)") { retainAll(listOf("d", "e")) },
CollectionOperation("retainAll(emptyList())") { retainAll(emptyList()) },
CollectionOperation("clear()") { clear() },
CollectionOperation("iterator.remove()") { iterator().apply { next(); remove() } },
)
fun testThrowsCME(withMutableSet: WithCollection<MutableSet<String>>) {
for (setOp in operations) {
for (iteratorOp in iteratorOperations<String>()) {
testThrowsCME(withMutableSet, { iterator() }, setOp, iteratorOp)
}
}
}
// Because platform implementations may have different load factor and rehash strategy,
// make sure there is enough capacity to avoid rehash.
val elements = listOf("a", "b", "c", "d")
testThrowsCME { action ->
LinkedHashSet<String>(10).apply {
addAll(elements)
action(this)
}
}
testThrowsCME { action ->
HashSet<String>(10).apply {
addAll(elements)
action(this)
}
}
testThrowsCME { action ->
buildSet(10) {
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") },
CollectionOperation("put(update value)", throwsCME = false) { put("d", "D") },
CollectionOperation("remove(non-existing)", throwsCME = false) { remove("e") },
CollectionOperation("remove(existing)") { remove("d") },
CollectionOperation("putAll(emptyList())", throwsCME = false) { putAll(emptyMap()) },
CollectionOperation("putAll(existing)", throwsCME = false) { putAll(mapOf("d" to "d", "b" to "b")) },
CollectionOperation("putAll(update values)", throwsCME = false) { putAll(mapOf("d" to "D", "b" to "B")) },
CollectionOperation("putAll(some exist)") { putAll(mapOf("d" to "d", "e" to "e")) },
CollectionOperation("putAll(non-existing)") { putAll(mapOf("e" to "e", "f" to "f")) },
CollectionOperation("clear()") { clear() },
CollectionOperation("iterator.remove()") { iterator().apply { next(); remove() } },
)
fun testThrowsCME(withMutableMap: WithCollection<MutableMap<String, String>>) {
for (mapOp in operations) {
for (iteratorOp in iteratorOperations<String>()) {
testThrowsCME(withMutableMap, { keys.iterator() }, mapOp, iteratorOp)
testThrowsCME(withMutableMap, { values.iterator() }, mapOp, iteratorOp)
}
for (iteratorOp in iteratorOperations<MutableMap.MutableEntry<String, String>>()) {
testThrowsCME(withMutableMap, { entries.iterator() }, mapOp, iteratorOp)
}
}
}
// Because platform implementations may have different load factor and rehash strategy,
// make sure there is enough capacity to avoid rehash.
val entries = mapOf("a" to "a", "b" to "b", "c" to "c", "d" to "d")
testThrowsCME { action ->
LinkedHashMap<String, String>(10).apply {
putAll(entries)
action(this)
}
}
testThrowsCME { action ->
HashMap<String, String>(10).apply {
putAll(entries)
action(this)
}
}
testThrowsCME { action ->
buildMap(10) {
putAll(entries)
action(this)
}
}
}
}
private typealias WithCollection<C> = (action: (C) -> Unit) -> Unit