Skip to content
This repository was archived by the owner on Sep 1, 2020. It is now read-only.

Commit 0c99742

Browse files
committed
SI-9365 Don't null out dependencies of transient lazy vals
As per Iulian's analysis: > When lazy vals are transient, the optimization in SI-720 is invalid, > leading to NPEs. These NPEs appear when recomputing the lazy val when deserializaing the object. This commit disables the field nulling if the lazy val is marked transient. The post-mixin tree in the enclosed test changes as follows: ``` --- sandbox/old.log 2015-07-27 15:48:03.000000000 +1000 +++ sandbox/new.log 2015-07-27 15:47:56.000000000 +1000 @@ -1,61 +1,54 @@ [[syntax trees at end of mixin]] // t9365.scala package <empty> { class Test extends Object with Serializable { @transient @volatile private[this] var bitmap$trans$0: Boolean = false; private def foo$lzycompute(): Object = { { Test.this.synchronized({ if (Test.this.bitmap$trans$0.unary_!()) { Test.this.foo = Test.this.x.apply(); Test.this.bitmap$trans$0 = true; () }; scala.runtime.BoxedUnit.UNIT }); - Test.this.x = null + () }; Test.this.foo }; ``` In addition to the test from the ticket, I've added a reflection based test that directly tests the nulling. This complements the test added in 449f2a7, the fix for SI-720, which passes by virtue of not exhausting the heap.
1 parent 0e9525a commit 0c99742

File tree

4 files changed

+69
-1
lines changed

4 files changed

+69
-1
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
11221122
if (scope exists (_.isLazy)) {
11231123
val map = mutable.Map[Symbol, Set[Symbol]]() withDefaultValue Set()
11241124
// check what fields can be nulled for
1125-
for ((field, users) <- singleUseFields(templ); lazyFld <- users)
1125+
for ((field, users) <- singleUseFields(templ); lazyFld <- users if !lazyFld.accessed.hasAnnotation(TransientAttr))
11261126
map(lazyFld) += field
11271127

11281128
map.toMap

test/files/run/t720.scala

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
class Lazy(f: => Int) {
2+
lazy val get: Int = f
3+
}
4+
5+
class UsedLater(f: => Int) {
6+
lazy val get: Int = f
7+
def other = f
8+
}
9+
10+
class TransientLazy(f: => Int) {
11+
@transient
12+
lazy val get: Int = f
13+
}
14+
15+
object Test {
16+
def main(args: Array[String]): Unit = {
17+
testLazy()
18+
testUsedLater()
19+
}
20+
21+
def testLazy() {
22+
val o = new Lazy("".length)
23+
val f = classOf[Lazy].getDeclaredField("f")
24+
f.setAccessible(true)
25+
assert(f.get(o) != null)
26+
o.get
27+
assert(f.get(o) == null)
28+
}
29+
30+
def testUsedLater() {
31+
val o = new UsedLater("".length)
32+
val f = classOf[UsedLater].getDeclaredField("f")
33+
f.setAccessible(true)
34+
assert(f.get(o) != null)
35+
o.get
36+
assert(f.get(o) != null)
37+
}
38+
39+
def testTransientLazy() {
40+
val o = new TransientLazy("".length)
41+
val f = classOf[TransientLazy].getDeclaredField("f")
42+
f.setAccessible(true)
43+
assert(f.get(o) != null)
44+
o.get
45+
assert(f.get(o) != null) // SI-9365
46+
}
47+
}
48+

test/files/run/t9365.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
foo
2+
foo

test/files/run/t9365.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class Test(x: => Object) extends Serializable {
2+
@transient lazy val foo = x
3+
}
4+
5+
object Test {
6+
def main(args: Array[String]): Unit = {
7+
import java.io._
8+
val t = new Test("foo")
9+
println(t.foo)
10+
val baos = new ByteArrayOutputStream
11+
val dos = new ObjectOutputStream(baos)
12+
dos.writeObject(t)
13+
dos.close()
14+
val dis = new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))
15+
val t1 = dis.readObject().asInstanceOf[Test]
16+
println(t1.foo) // was NPE
17+
}
18+
}

0 commit comments

Comments
 (0)