Skip to content

Commit

Permalink
SI-7775 Harden against the shifting sands of System.getProperties
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
retronym committed Aug 26, 2013
1 parent 7b351dc commit 9d5ed33
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/compiler/scala/tools/reflect/WrappedProperties.scala
Expand Up @@ -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
}
}

Expand Down
17 changes: 17 additions & 0 deletions 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)
}
}

0 comments on commit 9d5ed33

Please sign in to comment.