From defb1465909c3f740871a56973c32b276f775b91 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Wed, 29 Jul 2015 11:39:04 +0200 Subject: SI-9375 add synthetic readResolve only for static modules For inner modules, the synthetic readResolve method would cause the module constructor to be invoked on de-serialization in certain situations. See the discussion in the ticket. Adds a comprehensive test around serializing and de-serializing modules. --- .../tools/nsc/typechecker/SyntheticMethods.scala | 1 + test/files/neg/t6666d.check | 4 - test/files/neg/t6666d.scala | 18 -- test/files/pos/t6666d.scala | 18 ++ test/files/run/idempotency-case-classes.check | 3 +- test/files/run/repl-serialization.check | 1 - test/files/run/t9375.check | 60 +++++ test/files/run/t9375.scala | 276 +++++++++++++++++++++ 8 files changed, 356 insertions(+), 25 deletions(-) delete mode 100644 test/files/neg/t6666d.check delete mode 100644 test/files/neg/t6666d.scala create mode 100644 test/files/pos/t6666d.scala create mode 100644 test/files/run/t9375.check create mode 100644 test/files/run/t9375.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala index c156b8c677..4ccc183334 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala @@ -304,6 +304,7 @@ trait SyntheticMethods extends ast.TreeDSL { clazz.isModuleClass && clazz.isSerializable && !hasConcreteImpl(nme.readResolve) + && clazz.isStatic ) def synthesize(): List[Tree] = { diff --git a/test/files/neg/t6666d.check b/test/files/neg/t6666d.check deleted file mode 100644 index b4785f0129..0000000000 --- a/test/files/neg/t6666d.check +++ /dev/null @@ -1,4 +0,0 @@ -t6666d.scala:7: error: Implementation restriction: access of object TreeOrd$1 from object TreeOrd$2, would require illegal premature access to the unconstructed `this` of class Test - implicit object TreeOrd extends Ordering[K](){ - ^ -one error found diff --git a/test/files/neg/t6666d.scala b/test/files/neg/t6666d.scala deleted file mode 100644 index 49a688f91b..0000000000 --- a/test/files/neg/t6666d.scala +++ /dev/null @@ -1,18 +0,0 @@ - -import scala.collection.immutable.TreeMap -import scala.math.Ordering - -class Test[K](param:TreeMap[K,Int]){ - def this() = this({ - implicit object TreeOrd extends Ordering[K](){ - def compare(a: K, b: K) = { - -1 - } - } - new TreeMap[K, Int]() - }) -} - -object Test extends App { - new Test() -} diff --git a/test/files/pos/t6666d.scala b/test/files/pos/t6666d.scala new file mode 100644 index 0000000000..49a688f91b --- /dev/null +++ b/test/files/pos/t6666d.scala @@ -0,0 +1,18 @@ + +import scala.collection.immutable.TreeMap +import scala.math.Ordering + +class Test[K](param:TreeMap[K,Int]){ + def this() = this({ + implicit object TreeOrd extends Ordering[K](){ + def compare(a: K, b: K) = { + -1 + } + } + new TreeMap[K, Int]() + }) +} + +object Test extends App { + new Test() +} diff --git a/test/files/run/idempotency-case-classes.check b/test/files/run/idempotency-case-classes.check index 5a8d0ad9d3..ea698cec59 100644 --- a/test/files/run/idempotency-case-classes.check +++ b/test/files/run/idempotency-case-classes.check @@ -47,8 +47,7 @@ C(2,3) case def unapply(x$0: C): Option[(Int, Int)] = if (x$0.==(null)) scala.this.None else - Some.apply[(Int, Int)](scala.Tuple2.apply[Int, Int](x$0.x, x$0.y)); - private def readResolve(): Object = C + Some.apply[(Int, Int)](scala.Tuple2.apply[Int, Int](x$0.x, x$0.y)) }; Predef.println(C.apply(2, 3)) } diff --git a/test/files/run/repl-serialization.check b/test/files/run/repl-serialization.check index eb62729f5c..bbbf0dcdf1 100644 --- a/test/files/run/repl-serialization.check +++ b/test/files/run/repl-serialization.check @@ -20,6 +20,5 @@ u: U = U evaluating O constructing A == reconstituting into a fresh classloader - evaluating O == evaluating reconstituted lambda constructing A diff --git a/test/files/run/t9375.check b/test/files/run/t9375.check new file mode 100644 index 0000000000..8f43fab025 --- /dev/null +++ b/test/files/run/t9375.check @@ -0,0 +1,60 @@ + konstruktor: class A + konstruktor: class A$O$12$ + konstruktor: class A$$anon$1 + konstruktor: class A$A + konstruktor: class A$C + konstruktor: class C + konstruktor: class T$O$15$ + konstruktor: class T$$anon$2 + konstruktor: class T$A + konstruktor: class T$C + konstruktor: class A$N$ + konstruktor: class T$N$ +serializing outer objects should not initialize any nested objects +now initializing nested objects + konstruktor: class A$O$ + konstruktor: class A$Op$ + konstruktor: class A$N$O$ + konstruktor: class A$N$Op$ + konstruktor: class A$A$O$ + konstruktor: class A$A$Op$ + konstruktor: class A$T$O$ + konstruktor: class A$T$Op$ + konstruktor: class A$O$11$ + konstruktor: class A$O$13$ + konstruktor: class A$$anon$1$O$ + konstruktor: class A$$anon$1$Op$ + konstruktor: class T$O$ + konstruktor: class T$Op$ + konstruktor: class T$N$O$ + konstruktor: class T$N$Op$ + konstruktor: class T$A$O$ + konstruktor: class T$A$Op$ + konstruktor: class T$T$O$ + konstruktor: class T$T$Op$ + konstruktor: class T$O$14$ + konstruktor: class T$O$16$ + konstruktor: class T$$anon$2$O$ + konstruktor: class T$$anon$2$Op$ +no object konstruktors called when serializing / deserializing objects (starting at the outer or the object itself) +deserializing outer objects with non-initialized inners again +accessing modules triggers initialization + konstruktor: class A$O$ + konstruktor: class A$Op$ + konstruktor: class A$N$O$ + konstruktor: class A$N$Op$ +deserializing creates a new object graph, including new scala 'object' instances, no matter where serialization starts +init static module M and field v + konstruktor: class M$ + konstruktor: class M$O$18$ +serDeser does not initialize nested static modules +init M.O + konstruktor: class M$O$ +serDeser nested static module +objects declared in field decls are not static modules, so they deserialize to new instances +init lazy val M.w +objects declared in lazy val are not static modules either + konstruktor: class M$O$19$ +object declared in a function: new instance created on each invocation + konstruktor: class M$O$20$ + konstruktor: class M$O$20$ diff --git a/test/files/run/t9375.scala b/test/files/run/t9375.scala new file mode 100644 index 0000000000..3995b38666 --- /dev/null +++ b/test/files/run/t9375.scala @@ -0,0 +1,276 @@ +import java.io._ + +object SerDes { + def serialize(obj: AnyRef): Array[Byte] = { + val buffer = new ByteArrayOutputStream + val out = new ObjectOutputStream(buffer) + out.writeObject(obj) + buffer.toByteArray + } + + def deserialize(a: Array[Byte]): AnyRef = { + val in = new ObjectInputStream(new ByteArrayInputStream(a)) + in.readObject + } + + def serializeDeserialize[T <: AnyRef](obj: T) = deserialize(serialize(obj)).asInstanceOf[T] +} + +import SerDes._ + +// tests to make sure that de-serializing an object does not run its constructor + +trait S extends Serializable { + println(" konstruktor: " + this.getClass) +} + +trait SE extends S { + def outer: Object +} + +class A extends S { + object O extends SE { def outer = A.this } + private[this] object Op extends SE { def outer = A.this } + def P: SE = Op + + object N extends S { + object O extends SE { def outer = N } + private[this] object Op extends SE { def outer = N } + def P: SE = Op + } + + class A extends S { + object O extends SE { def outer = A.this } + private[this] object Op extends SE { def outer = A.this } + def P: SE = Op + } + + trait T extends S { + object O extends SE { def outer = T.this } + private[this] object Op extends SE { def outer = T.this } + def P: SE = Op + } + class C extends T + + def u: SE = { + object O extends SE { def outer = A.this } + O + } + + val v: SE = { + object O extends SE { def outer = A.this } + O + } + + val f: () => SE = () => { + object O extends SE { def outer = A.this } + O + } + + trait GetObj { def O: SE; def P: SE } + val a: GetObj = new GetObj with S { + def anonThis = this + object O extends SE { def outer = anonThis } + private[this] object Op extends SE { def outer = anonThis } + def P: SE = Op + } +} + +trait T extends S { + object O extends SE { def outer = T.this } + private[this] object Op extends SE { def outer = T.this } + def P: SE = Op + + object N extends S { + object O extends SE { def outer = N } + private[this] object Op extends SE { def outer = N } + def P: SE = Op + } + + class A extends S { + object O extends SE { def outer = A.this } + private[this] object Op extends SE { def outer = A.this } + def P: SE = Op + } + + trait T extends S { + object O extends SE { def outer = T.this } + private[this] object Op extends SE { def outer = T.this } + def P: SE = Op + } + class C extends T + + def u: SE = { + object O extends SE { def outer = T.this } + O + } + + val v: SE = { + object O extends SE { def outer = T.this } + O + } + + val f: () => SE = () => { + object O extends SE { def outer = T.this } + O + } + + trait GetObj { def O: SE; def P: SE } + val a: GetObj = new GetObj with S { + def anonThis = this + object O extends SE { def outer = anonThis } + private[this] object Op extends SE { def outer = anonThis } + def P: SE = Op + } +} + +class C extends T + +object DeserializeModuleNoConstructor { + def t(): Unit = { + val a = new A + val aa = new a.A + val ac = new a.C + + val c = new C + val ca = new c.A + val cc = new c.C + + val outers: List[Object] = List( + a, a.N, aa, ac, a.a, + c, c.N, ca, cc, c.a + ) + + println("serializing outer objects should not initialize any nested objects") + + val serANotInit = serialize(a) + outers foreach serializeDeserialize + + println("now initializing nested objects") + + val os: List[(SE, Object)] = List( + a.O -> a, + a.P -> a, + a.N.O -> a.N, + a.N.P -> a.N, + aa.O -> aa, + aa.P -> aa, + ac.O -> ac, + ac.P -> ac, + a.u -> a, + a.v -> a, + a.f() -> a, + a.a.O -> a.a, + a.a.P -> a.a, + + c.O -> c, + c.P -> c, + c.N.O -> c.N, + c.N.P -> c.N, + ca.O -> ca, + ca.P -> ca, + cc.O -> cc, + cc.P -> cc, + c.u -> c, + c.v -> c, + c.f() -> c, + c.a.O -> c.a, + c.a.P -> c.a + ) + + println("no object konstruktors called when serializing / deserializing objects (starting at the outer or the object itself)") + + for ((obj, outer) <- os) { + assert(obj.outer eq outer, s"${obj.outer} of $obj -- $outer") + serializeDeserialize(obj) + serializeDeserialize(outer) + } + + println("deserializing outer objects with non-initialized inners again") + val aNotInit = deserialize(serANotInit).asInstanceOf[A] + + println("accessing modules triggers initialization") + aNotInit.O + aNotInit.P + aNotInit.N.O + aNotInit.N.P + + println("deserializing creates a new object graph, including new scala 'object' instances, no matter where serialization starts") + val deserializedAs: List[A] = List( + serializeDeserialize(a), + serializeDeserialize(a.O).outer.asInstanceOf[A], + serializeDeserialize(a.P).outer.asInstanceOf[A], + serializeDeserialize(a.v).outer.asInstanceOf[A] + ) + for (aSD <- deserializedAs) { + assert(aSD ne a) + assert(aSD.O ne a.O) + assert(aSD.P ne a.P) + assert(aSD.N ne a.N) + assert(aSD.N.O ne a.N.O) + assert(aSD.N.P ne a.N.P) + assert(aSD.v ne a.v) + assert(aSD.a.O ne a.a.O) + assert(aSD.a.P ne a.a.P) + } + } +} + +// tests for serializing / deserializing static modules + +object M extends S { + object O extends S + + def u: S = { + object O extends S + O + } + + val v: S = { + object O extends S + O + } + + lazy val w: S = { + object O extends S + O + } + + val f: () => S = () => { + object O extends S + O + } +} + +object SerializingStaticModules { + def t(): Unit = { + println("init static module M and field v") + M + + println("serDeser does not initialize nested static modules") + assert(serializeDeserialize(M) eq M) + + println("init M.O") + M.O + + println("serDeser nested static module") + assert(serializeDeserialize(M.O) eq M.O) + + println("objects declared in field decls are not static modules, so they deserialize to new instances") + assert(serializeDeserialize(M.v) ne M.v) + + println("init lazy val M.w") + + println("objects declared in lazy val are not static modules either") + assert(serializeDeserialize(M.w) ne M.w) + + println("object declared in a function: new instance created on each invocation") + assert(M.f() ne M.f()) + } +} + + +object Test extends App { + DeserializeModuleNoConstructor.t() + SerializingStaticModules.t() +} -- cgit v1.2.3