Skip to content

Commit 4c627b7

Browse files
committed
[backport] 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.
1 parent 53d4ec0 commit 4c627b7

File tree

3 files changed

+47
-7
lines changed

3 files changed

+47
-7
lines changed

src/compiler/scala/tools/nsc/transform/CleanUp.scala

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,17 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
227227
var reflPoly$Cache: SoftReference[scala.runtime.MethodCache] = new SoftReference(new EmptyMethodCache())
228228
229229
def reflMethod$Method(forReceiver: JClass[_]): JMethod = {
230-
var method: JMethod = reflPoly$Cache.find(forReceiver)
231-
if (method != null)
230+
var methodCache: MethodCache = reflPoly$Cache.find(forReceiver)
231+
if (methodCache eq null) {
232+
methodCache = new EmptyMethodCache
233+
reflPoly$Cache = new SoftReference(methodCache)
234+
}
235+
var method: JMethod = methodCache.find(forReceiver)
236+
if (method ne null)
232237
return method
233238
else {
234239
method = ScalaRunTime.ensureAccessible(forReceiver.getMethod("xyz", reflParams$Cache))
235-
reflPoly$Cache = new SoftReference(reflPoly$Cache.get.add(forReceiver, method))
240+
reflPoly$Cache = new SoftReference(methodCache.add(forReceiver, method))
236241
return method
237242
}
238243
}
@@ -248,16 +253,22 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
248253

249254
addStaticMethodToClass("reflMethod$Method", List(ClassClass.tpe), MethodClass.tpe)
250255
{ case Pair(reflMethodSym, List(forReceiverSym)) =>
256+
val methodCache = reflMethodSym.newVariable(ad.pos, mkTerm("methodCache")) setInfo MethodCacheClass.tpe
251257
val methodSym = reflMethodSym.newVariable(ad.pos, mkTerm("method")) setInfo MethodClass.tpe
252258

253259
BLOCK(
254-
IF (getPolyCache OBJ_EQ NULL) THEN (safeREF(reflPolyCacheSym) === mkNewPolyCache) ENDIF,
255-
VAL(methodSym) === ((getPolyCache DOT methodCache_find)(REF(forReceiverSym))) ,
256-
IF (REF(methodSym) OBJ_!= NULL) .
260+
VAR(methodCache) === getPolyCache,
261+
IF (REF(methodCache) OBJ_EQ NULL) THEN BLOCK(
262+
REF(methodCache) === NEW(TypeTree(EmptyMethodCacheClass.tpe)),
263+
REF(reflPolyCacheSym) === gen.mkSoftRef(REF(methodCache))
264+
) ENDIF,
265+
266+
VAR(methodSym) === (REF(methodCache) DOT methodCache_find)(REF(forReceiverSym)),
267+
IF (REF(methodSym) OBJ_NE NULL) .
257268
THEN (Return(REF(methodSym)))
258269
ELSE {
259270
def methodSymRHS = ((REF(forReceiverSym) DOT Class_getMethod)(LIT(method), safeREF(reflParamsCacheSym)))
260-
def cacheRHS = ((getPolyCache DOT methodCache_add)(REF(forReceiverSym), REF(methodSym)))
271+
def cacheRHS = ((REF(methodCache) DOT methodCache_add)(REF(forReceiverSym), REF(methodSym)))
261272
BLOCK(
262273
REF(methodSym) === (REF(ensureAccessibleMethod) APPLY (methodSymRHS)),
263274
safeREF(reflPolyCacheSym) === gen.mkSoftRef(cacheRHS),

test/files/run/t6969.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
All threads completed.

test/files/run/t6969.scala

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
object Test {
2+
private type Clearable = { def clear(): Unit }
3+
private def choke() = {
4+
try new Array[Object]((Runtime.getRuntime().maxMemory min Int.MaxValue).toInt)
5+
catch {
6+
case _: OutOfMemoryError => // what do you mean, out of memory?
7+
case t: Throwable => println(t)
8+
}
9+
}
10+
private def f(x: Clearable) = x.clear()
11+
class Choker(id: Int) extends Thread {
12+
private def g(iteration: Int) = {
13+
val map = scala.collection.mutable.Map[Int, Int](1 -> 2)
14+
try f(map) catch { case t: NullPointerException => println("Failed at "+id+"/"+iteration) ; throw t }
15+
choke()
16+
}
17+
override def run() {
18+
1 to 50 foreach g
19+
}
20+
}
21+
22+
def main(args: Array[String]): Unit = {
23+
val threads = 1 to 3 map (id => new Choker(id))
24+
threads foreach (_.start())
25+
threads foreach (_.join())
26+
println("All threads completed.")
27+
}
28+
}

0 commit comments

Comments
 (0)