diff options
author | Rex Kerr <ichoran@gmail.com> | 2014-06-27 17:12:39 -0700 |
---|---|---|
committer | Rex Kerr <ichoran@gmail.com> | 2014-06-27 21:59:47 -0700 |
commit | 9ceab680a315cb79d4d187f977e9dac3b58e48c0 (patch) | |
tree | dc09eac8728e1decae16ca62cb38a060e43a928a | |
parent | fbfce33cb03bc2b41dd0f46fa9f4630036b4f2ca (diff) | |
download | scala-9ceab680a315cb79d4d187f977e9dac3b58e48c0.tar.gz scala-9ceab680a315cb79d4d187f977e9dac3b58e48c0.tar.bz2 scala-9ceab680a315cb79d4d187f977e9dac3b58e48c0.zip |
SI-7115 JMapWrapper.get can incorrectly return Some(null)
This isn't incorrect. Trying to use a single-threaded interface in a concurrent context is supposed to break in various unpleasant ways.
Documentation has been added to encourage one to avoid wrapping a concurrent map in the generic wrapper (which assumes a single thread), and pointing out that synchronized maps do not maintain synchronization for non-atomic operations (including get).
More docs.
-rw-r--r-- | src/library/scala/collection/convert/DecorateAsScala.scala | 6 | ||||
-rw-r--r-- | src/library/scala/collection/convert/WrapAsScala.scala | 8 | ||||
-rw-r--r-- | src/library/scala/collection/convert/Wrappers.scala | 11 |
3 files changed, 24 insertions, 1 deletions
diff --git a/src/library/scala/collection/convert/DecorateAsScala.scala b/src/library/scala/collection/convert/DecorateAsScala.scala index c724831c54..5448f5f91c 100644 --- a/src/library/scala/collection/convert/DecorateAsScala.scala +++ b/src/library/scala/collection/convert/DecorateAsScala.scala @@ -135,6 +135,12 @@ trait DecorateAsScala { * If the Java `Map` was previously obtained from an implicit or explicit * call of `asMap(scala.collection.mutable.Map)` then the original * Scala `Map` will be returned. + * + * If the wrapped map is synchronized (e.g. from `java.util.Collections.synchronizedMap`), + * it is your responsibility to wrap all + * non-atomic operations with `underlying.synchronized`. + * This includes `get`, as `java.util.Map`'s API does not allow for an + * atomic `get` when `null` values may be present. * * @param m The `Map` to be converted. * @return An object with an `asScala` method that returns a Scala mutable diff --git a/src/library/scala/collection/convert/WrapAsScala.scala b/src/library/scala/collection/convert/WrapAsScala.scala index d4ab451b0d..ab151a6778 100644 --- a/src/library/scala/collection/convert/WrapAsScala.scala +++ b/src/library/scala/collection/convert/WrapAsScala.scala @@ -133,7 +133,13 @@ trait WrapAsScala { * If the Java `Map` was previously obtained from an implicit or * explicit call of `mapAsScalaMap(scala.collection.mutable.Map)` then * the original Scala Map will be returned. - * + * + * If the wrapped map is synchronized (e.g. from `java.util.Collections.synchronizedMap`), + * it is your responsibility to wrap all + * non-atomic operations with `underlying.synchronized`. + * This includes `get`, as `java.util.Map`'s API does not allow for an + * atomic `get` when `null` values may be present. + * * @param m The Map to be converted. * @return A Scala mutable Map view of the argument. */ diff --git a/src/library/scala/collection/convert/Wrappers.scala b/src/library/scala/collection/convert/Wrappers.scala index 7d1d6b3781..9f9732c62f 100644 --- a/src/library/scala/collection/convert/Wrappers.scala +++ b/src/library/scala/collection/convert/Wrappers.scala @@ -288,6 +288,13 @@ private[collection] trait Wrappers { override def empty: Repr = null.asInstanceOf[Repr] } + /** Wraps a Java map as a Scala one. If the map is to support concurrent access, + * use [[JConcurrentMapWrapper]] instead. If the wrapped map is synchronized + * (e.g. from `java.util.Collections.synchronizedMap`), it is your responsibility + * to wrap all non-atomic operations with `underlying.synchronized`. + * This includes `get`, as `java.util.Map`'s API does not allow for an + * atomic `get` when `null` values may be present. + */ case class JMapWrapper[A, B](underlying : ju.Map[A, B]) extends mutable.AbstractMap[A, B] with JMapWrapperLike[A, B, JMapWrapper[A, B]] { override def empty = JMapWrapper(new ju.HashMap[A, B]) } @@ -314,6 +321,10 @@ private[collection] trait Wrappers { def replace(k: A, oldval: B, newval: B) = underlying.replace(k, oldval, newval) } + /** Wraps a concurrent Java map as a Scala one. Single-element concurrent + * access is supported; multi-element operations such as maps and filters + * are not guaranteed to be atomic. + */ case class JConcurrentMapWrapper[A, B](underlying: juc.ConcurrentMap[A, B]) extends mutable.AbstractMap[A, B] with JMapWrapperLike[A, B, JConcurrentMapWrapper[A, B]] with concurrent.Map[A, B] { override def get(k: A) = { val v = underlying get k |