diff options
author | Rex Kerr <ichoran@gmail.com> | 2014-01-30 11:43:26 -0800 |
---|---|---|
committer | Rex Kerr <ichoran@gmail.com> | 2014-01-30 11:47:05 -0800 |
commit | f97e2d42eb211d429b27f79fe993bf48c92e9740 (patch) | |
tree | 876b10816206ff8180e572804bb101464f560b2c | |
parent | 0e578e693196f93b1ba4f972a2c96d468bef464a (diff) | |
download | scala-f97e2d42eb211d429b27f79fe993bf48c92e9740.tar.gz scala-f97e2d42eb211d429b27f79fe993bf48c92e9740.tar.bz2 scala-f97e2d42eb211d429b27f79fe993bf48c92e9740.zip |
SI-8213 AnyRefMap.getOrElseUpdate is faulty
Altered getOrElseUpdate to be robust to the map changing out from under it as a result of calling the default value method. Side-effects FTW!
Made a comparable change in LongMap also, as it was also affected. And added a test to SetMapConsistencyTest.
-rw-r--r-- | src/library/scala/collection/mutable/AnyRefMap.scala | 15 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/LongMap.scala | 15 | ||||
-rw-r--r-- | test/junit/scala/collection/SetMapConsistencyTest.scala | 12 |
3 files changed, 38 insertions, 4 deletions
diff --git a/src/library/scala/collection/mutable/AnyRefMap.scala b/src/library/scala/collection/mutable/AnyRefMap.scala index df74bb5187..29c92a111c 100644 --- a/src/library/scala/collection/mutable/AnyRefMap.scala +++ b/src/library/scala/collection/mutable/AnyRefMap.scala @@ -129,9 +129,20 @@ extends AbstractMap[K, V] override def getOrElseUpdate(key: K, defaultValue: => V): V = { val h = hashOf(key) - val i = seekEntryOrOpen(h, key) + var i = seekEntryOrOpen(h, key) if (i < 0) { - val value = defaultValue + // It is possible that the default value computation was side-effecting + // Our hash table may have resized or even contain what we want now + // (but if it does, we'll replace it) + val value = { + val oh = _hashes + val ans = defaultValue + if (oh ne _hashes) { + i = seekEntryOrOpen(h, key) + if (i >= 0) _size -= 1 + } + ans + } _size += 1 val j = i & IndexMask _hashes(j) = h diff --git a/src/library/scala/collection/mutable/LongMap.scala b/src/library/scala/collection/mutable/LongMap.scala index 81c381279f..984ae6f7cc 100644 --- a/src/library/scala/collection/mutable/LongMap.scala +++ b/src/library/scala/collection/mutable/LongMap.scala @@ -160,9 +160,20 @@ extends AbstractMap[Long, V] else minValue.asInstanceOf[V] } else { - val i = seekEntryOrOpen(key) + var i = seekEntryOrOpen(key) if (i < 0) { - val value = defaultValue + // It is possible that the default value computation was side-effecting + // Our hash table may have resized or even contain what we want now + // (but if it does, we'll replace it) + val value = { + val ok = _keys + val ans = defaultValue + if (ok ne _keys) { + i = seekEntryOrOpen(key) + if (i >= 0) _size -= 1 + } + ans + } _size += 1 val j = i & IndexMask _keys(j) = key diff --git a/test/junit/scala/collection/SetMapConsistencyTest.scala b/test/junit/scala/collection/SetMapConsistencyTest.scala index c62b074483..7bb8ca958b 100644 --- a/test/junit/scala/collection/SetMapConsistencyTest.scala +++ b/test/junit/scala/collection/SetMapConsistencyTest.scala @@ -476,4 +476,16 @@ class SetMapConsistencyTest { } assert(test) } + + @Test + def si8213() { + val am = new scala.collection.mutable.AnyRefMap[String, Int] + for (i <- 0 until 1024) am += i.toString -> i + am.getOrElseUpdate("1024", { am.clear; -1 }) + assert(am == scala.collection.mutable.AnyRefMap("1024" -> -1)) + val lm = new scala.collection.mutable.LongMap[Int] + for (i <- 0 until 1024) lm += i.toLong -> i + lm.getOrElseUpdate(1024, { lm.clear; -1 }) + assert(lm == scala.collection.mutable.LongMap(1024L -> -1)) + } } |