diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-11-26 21:27:59 +1000 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-11-26 21:27:59 +1000 |
commit | b1b5b90347869a79af7845316548ad40e5fbb911 (patch) | |
tree | 8d13a3012433835282bcdb0287052c2408296b04 | |
parent | b6fcb5a38d4a4ff452b11f949f11b18f42896fff (diff) | |
parent | d367c5f925e7055e9ee5b4071328e92cf48ce0ed (diff) | |
download | scala-b1b5b90347869a79af7845316548ad40e5fbb911.tar.gz scala-b1b5b90347869a79af7845316548ad40e5fbb911.tar.bz2 scala-b1b5b90347869a79af7845316548ad40e5fbb911.zip |
Merge pull request #4148 from SpinGo/issue/SI-8946
SI-8946: Fixes memory leak when using reflection
-rw-r--r-- | bincompat-backward.whitelist.conf | 5 | ||||
-rw-r--r-- | src/reflect/scala/reflect/runtime/ThreadLocalStorage.scala | 8 | ||||
-rw-r--r-- | test/files/run/t8946.scala | 29 |
3 files changed, 40 insertions, 2 deletions
diff --git a/bincompat-backward.whitelist.conf b/bincompat-backward.whitelist.conf index 076b9bb9aa..56d5b0135c 100644 --- a/bincompat-backward.whitelist.conf +++ b/bincompat-backward.whitelist.conf @@ -198,6 +198,11 @@ filter { { matchName="scala.collection.immutable.Stream.scala$collection$immutable$Stream$$loop$4" problemName=MissingMethodProblem + }, + // SI-8946 + { + matchName="scala.reflect.runtime.ThreadLocalStorage#MyThreadLocalStorage.values" + problemName=MissingMethodProblem } ] } diff --git a/src/reflect/scala/reflect/runtime/ThreadLocalStorage.scala b/src/reflect/scala/reflect/runtime/ThreadLocalStorage.scala index 5edc051461..586b8a5257 100644 --- a/src/reflect/scala/reflect/runtime/ThreadLocalStorage.scala +++ b/src/reflect/scala/reflect/runtime/ThreadLocalStorage.scala @@ -11,12 +11,16 @@ private[reflect] trait ThreadLocalStorage { trait ThreadLocalStorage[T] { def get: T; def set(newValue: T): Unit } private class MyThreadLocalStorage[T](initialValue: => T) extends ThreadLocalStorage[T] { // TODO: how do we use org.cliffc.high_scale_lib.NonBlockingHashMap here? - val values = new java.util.concurrent.ConcurrentHashMap[Thread, T]() + // (we would need a version that uses weak keys) + private val values = java.util.Collections.synchronizedMap(new java.util.WeakHashMap[Thread, T]()) def get: T = { if (values containsKey currentThread) values.get(currentThread) else { val value = initialValue - values.putIfAbsent(currentThread, value) + // since the key is currentThread, and `values` is private, it + // would be impossible for a value to have been set after the + // above containsKey check. `putIfAbsent` is not necessary. + values.put(currentThread, value) value } } diff --git a/test/files/run/t8946.scala b/test/files/run/t8946.scala new file mode 100644 index 0000000000..a248a20501 --- /dev/null +++ b/test/files/run/t8946.scala @@ -0,0 +1,29 @@ +// Tests to assert that references to threads are not strongly held when scala-reflection is used inside of them. +object Test { + import scala.ref.WeakReference + + def forceGc() = { + var obj = new Object + val ref = new WeakReference(obj) + obj = null; + while(ref.get.nonEmpty) + Array.ofDim[Byte](16 * 1024 * 1024) + } + + def main(args: Array[String]): Unit = { + val threads = for (i <- (1 to 16)) yield { + val t = new Thread { + override def run(): Unit = { + import reflect.runtime.universe._ + typeOf[List[String]] <:< typeOf[Seq[_]] + } + } + t.start() + t.join() + WeakReference(t) + } + forceGc() + val nonGCdThreads = threads.filter(_.get.nonEmpty).length + assert(nonGCdThreads == 0, s"${nonGCdThreads} threads were retained; expected 0.") + } +} |