Permalink
Browse files

SI-6969, mishandling of SoftReferences in method cache.

More interesting to test than it was to fix. The soft
reference is now dereferenced once, the locally stored
underlying value ascertained to be non-null, and the
remainder of the references to the value use the local var.

The enclosed test reliably NPEs without this patch.
  • Loading branch information...
1 parent e7ecaa0 commit 0d01cc1c300b718afd1fe69d5030d36d8000e0cd @paulp paulp committed Jan 15, 2013
Showing with 48 additions and 7 deletions.
  1. +19 −7 src/compiler/scala/tools/nsc/transform/CleanUp.scala
  2. +1 −0 test/files/run/t6969.check
  3. +28 −0 test/files/run/t6969.scala
@@ -207,12 +207,17 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
var reflPoly$Cache: SoftReference[scala.runtime.MethodCache] = new SoftReference(new EmptyMethodCache())
def reflMethod$Method(forReceiver: JClass[_]): JMethod = {
- var method: JMethod = reflPoly$Cache.find(forReceiver)
- if (method != null)
+ var methodCache: MethodCache = reflPoly$Cache.find(forReceiver)
+ if (methodCache eq null) {
+ methodCache = new EmptyMethodCache
+ reflPoly$Cache = new SoftReference(methodCache)
+ }
+ var method: JMethod = methodCache.find(forReceiver)
+ if (method ne null)
return method
else {
method = ScalaRunTime.ensureAccessible(forReceiver.getMethod("xyz", reflParams$Cache))
- reflPoly$Cache = new SoftReference(reflPoly$Cache.get.add(forReceiver, method))
+ reflPoly$Cache = new SoftReference(methodCache.add(forReceiver, method))
return method
}
}
@@ -229,16 +234,22 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
def getPolyCache = gen.mkCast(fn(REF(reflPolyCacheSym), nme.get), MethodCacheClass.tpe)
addStaticMethodToClass((reflMethodSym, forReceiverSym) => {
+ val methodCache = reflMethodSym.newVariable(mkTerm("methodCache"), ad.pos) setInfo MethodCacheClass.tpe
val methodSym = reflMethodSym.newVariable(mkTerm("method"), ad.pos) setInfo MethodClass.tpe
BLOCK(
- IF (getPolyCache OBJ_EQ NULL) THEN (REF(reflPolyCacheSym) === mkNewPolyCache) ENDIF,
- VAL(methodSym) === ((getPolyCache DOT methodCache_find)(REF(forReceiverSym))) ,
- IF (REF(methodSym) OBJ_!= NULL) .
+ VAR(methodCache) === getPolyCache,
+ IF (REF(methodCache) OBJ_EQ NULL) THEN BLOCK(
+ REF(methodCache) === NEW(TypeTree(EmptyMethodCacheClass.tpe)),
+ REF(reflPolyCacheSym) === gen.mkSoftRef(REF(methodCache))
+ ) ENDIF,
+
+ VAR(methodSym) === (REF(methodCache) DOT methodCache_find)(REF(forReceiverSym)),
+ IF (REF(methodSym) OBJ_NE NULL) .
THEN (Return(REF(methodSym)))
ELSE {
def methodSymRHS = ((REF(forReceiverSym) DOT Class_getMethod)(LIT(method), REF(reflParamsCacheSym)))
- def cacheRHS = ((getPolyCache DOT methodCache_add)(REF(forReceiverSym), REF(methodSym)))
+ def cacheRHS = ((REF(methodCache) DOT methodCache_add)(REF(forReceiverSym), REF(methodSym)))
BLOCK(
REF(methodSym) === (REF(ensureAccessibleMethod) APPLY (methodSymRHS)),
REF(reflPolyCacheSym) === gen.mkSoftRef(cacheRHS),
@@ -247,6 +258,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
}
)
})
+
}
/* ### HANDLING METHODS NORMALLY COMPILED TO OPERATORS ### */
@@ -0,0 +1 @@
+All threads completed.
@@ -0,0 +1,28 @@
+object Test {
+ private type Clearable = { def clear(): Unit }
+ private def choke() = {
+ try new Array[Object]((Runtime.getRuntime().maxMemory min Int.MaxValue).toInt)
+ catch {
+ case _: OutOfMemoryError => // what do you mean, out of memory?
+ case t: Throwable => println(t)
+ }
+ }
+ private def f(x: Clearable) = x.clear()
+ class Choker(id: Int) extends Thread {
+ private def g(iteration: Int) = {
+ val map = scala.collection.mutable.Map[Int, Int](1 -> 2)
+ try f(map) catch { case t: NullPointerException => println(s"Failed at $id/$iteration") ; throw t }
+ choke()
+ }
+ override def run() {
+ 1 to 50 foreach g
+ }
+ }
+
+ def main(args: Array[String]): Unit = {
+ val threads = 1 to 3 map (id => new Choker(id))
+ threads foreach (_.start())
+ threads foreach (_.join())
+ println("All threads completed.")
+ }
+}

0 comments on commit 0d01cc1

Please sign in to comment.