diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2013-08-23 09:29:56 +0200 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2013-08-26 16:11:10 +0200 |
commit | 9d5ed33ef9b503f20506dbe3e410960069a99d0a (patch) | |
tree | d9a59a5b289a9fafda3bccc0a3486c060a72009b | |
parent | 7b351dca8458f599f5fafef4daa307351031ef06 (diff) | |
download | scala-9d5ed33ef9b503f20506dbe3e410960069a99d0a.tar.gz scala-9d5ed33ef9b503f20506dbe3e410960069a99d0a.tar.bz2 scala-9d5ed33ef9b503f20506dbe3e410960069a99d0a.zip |
SI-7775 Harden against the shifting sands of System.getProperties
If another thread writes a new system property (which can happen
in pretty innocuous code such as `new Date`!), the compiler startup
could fail with a `ConcurrentModificationException` as it iterated
all bindings in the properties map in search of a boot classpath
property for esoteric JVMs.
This commit uses `Properties#getStringProperties` to get a snapshot
of the keys that isn't backed by the live map, and iterates these
instead. That method will also limit us to bindings with String
values, which is all that we expect.
-rw-r--r-- | src/compiler/scala/tools/reflect/WrappedProperties.scala | 8 | ||||
-rw-r--r-- | test/files/run/t7775.scala | 17 |
2 files changed, 23 insertions, 2 deletions
diff --git a/src/compiler/scala/tools/reflect/WrappedProperties.scala b/src/compiler/scala/tools/reflect/WrappedProperties.scala index c34edb8ba0..7ce0171c0b 100644 --- a/src/compiler/scala/tools/reflect/WrappedProperties.scala +++ b/src/compiler/scala/tools/reflect/WrappedProperties.scala @@ -26,9 +26,13 @@ trait WrappedProperties extends PropertiesTrait { override def envOrElse(name: String, alt: String) = wrap(super.envOrElse(name, alt)) getOrElse alt override def envOrNone(name: String) = wrap(super.envOrNone(name)).flatten - def systemProperties: Iterator[(String, String)] = { + def systemProperties: List[(String, String)] = { import scala.collection.JavaConverters._ - wrap(System.getProperties.asScala.iterator) getOrElse Iterator.empty + wrap { + val props = System.getProperties + // SI-7269 Be careful to avoid `ConcurrentModificationException` if another thread modifies the properties map + props.stringPropertyNames().asScala.toList.map(k => (k, props.get(k).asInstanceOf[String])) + } getOrElse Nil } } diff --git a/test/files/run/t7775.scala b/test/files/run/t7775.scala new file mode 100644 index 0000000000..5fb0327611 --- /dev/null +++ b/test/files/run/t7775.scala @@ -0,0 +1,17 @@ +import scala.concurrent.{duration, future, Await, ExecutionContext} +import scala.tools.nsc.Settings +import ExecutionContext.Implicits.global + +// Was failing pretty regularly with a ConcurrentModificationException as +// WrappedProperties#systemProperties iterated directly over the mutable +// global system properties map. +object Test { + def main(args: Array[String]) { + val tries = 1000 // YMMV + val compiler = future { + for(_ <- 1 to tries) new Settings(_ => {}) + } + for(i <- 1 to tries * 10) System.setProperty(s"foo$i", i.toString) + Await.result(compiler, duration.Duration.Inf) + } +} |