Fix clash between picklers with different sharing

* Different generated picklers with opposite sharing strategies are created and only the first one is registered. The second one never gets initiliazed and reuses the first one. * Remove `freshName` since scala `2.10.0` doesn't support.
scala · Apr 15, 2016 · f44ae1e · f44ae1e
1 parent 0c83b97
commit f44ae1e
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 15 deletions.
diff --git a/core/src/main/scala/scala/pickling/generator/sourcegen.scala b/core/src/main/scala/scala/pickling/generator/sourcegen.scala
@@ -385,6 +385,7 @@ private[pickling]  trait SourceGenerator extends Macro with tags.FastTypeTagMacr
 
   val picklingPath = q"_root_.scala.pickling"
   val picklersRegistry = q"$picklingPath.internal.currentRuntime.picklers"
+  val generated = tq"$picklingPath.Generated"
 
   def lazyInit(lookup: c.Tree, key: c.Tree, tpe: c.Tree,
                notInitialized: c.TermName): c.Tree = {
@@ -416,7 +417,7 @@ private[pickling]  trait SourceGenerator extends Macro with tags.FastTypeTagMacr
   def generatePicklerClass[T: c.WeakTypeTag](picklerAst: PicklerAst): c.Tree = {
 
     val tpe = computeType[T]
-    val picklerName = c.freshName(TermName(syntheticBaseName(tpe) + "Pickler"))
+    val picklerName = c.fresh(TermName(syntheticBaseName(tpe) + "Pickler"))
     val createTagTree = super[FastTypeTagMacros].impl[T]
     val picklerType = tq"$picklingPath.Pickler[$tpe]"
     val key = q"$createTagTree.key"
@@ -440,17 +441,17 @@ private[pickling]  trait SourceGenerator extends Macro with tags.FastTypeTagMacr
   def generateUnpicklerClass[T: c.WeakTypeTag](unpicklerAst: UnpicklerAst): c.Tree = {
 
     val tpe = computeType[T]
-    val unpicklerName = c.freshName(TermName(syntheticBaseName(tpe) + "Unpickler"))
+    val unpicklerName = c.fresh(TermName(syntheticBaseName(tpe) + "Unpickler"))
     val createTagTree = super[FastTypeTagMacros].impl[T]
     val unpickleLogic = genUnpicklerLogic[T](unpicklerAst)
     val unpicklerType = tq"$picklingPath.Unpickler[$tpe]"
-    val genUnpicklerType = tq"$unpicklerType with $picklingPath.Generated"
+    val genUnpicklerType = tq"$unpicklerType with $generated"
     val key = q"$createTagTree.key"
     val lookup = q"$picklersRegistry.lookupExistingUnpickler"
 
     q"""
        _root_.scala.Predef.locally {
-          implicit object $unpicklerName extends $unpicklerType with $picklingPath.Generated
+          implicit object $unpicklerName extends $unpicklerType with $generated
               with $picklingPath.AutoRegisterUnpickler[$tpe] {
 
             lazy val tag: $picklingPath.FastTypeTag[$tpe] = $createTagTree
@@ -466,20 +467,19 @@ private[pickling]  trait SourceGenerator extends Macro with tags.FastTypeTagMacr
   def generatePicklerUnpicklerClass[T: c.WeakTypeTag](impl: PickleUnpickleImplementation): c.Tree = {
 
     val tpe = computeType[T]
-    val name = c.freshName(TermName(syntheticBaseName(tpe) + "PicklerUnpickler"))
+    val name = c.fresh(TermName(syntheticBaseName(tpe) + "PicklerUnpickler"))
     val createTagTree = super[FastTypeTagMacros].impl[T]
     val unpickleLogic = genUnpicklerLogic[T](impl.unpickle)
     val pickleLogic = genPicklerLogic[T](impl.pickle)
     val key = q"$createTagTree.key"
     val picklerUnpicklerType = tq"$picklingPath.AbstractPicklerUnpickler[$tpe]"
-    val genPicklerUnpicklerType = tq"$picklerUnpicklerType with $picklingPath.Generated"
+    val genPicklerUnpicklerType = tq"$picklerUnpicklerType with $generated"
     val lookup1 = q"$picklersRegistry.lookupExistingPickler"
     val lookup2 = q"$picklersRegistry.lookupExistingUnpickler"
-    val castedName = c.freshName(TermName("castedName"))
 
     q"""
        _root_.scala.Predef.locally {
-          implicit object $name extends $picklerUnpicklerType with $picklingPath.Generated
+          implicit object $name extends $picklerUnpicklerType with $generated
               with $picklingPath.AutoRegister[$tpe] {
 
             override lazy val tag: $picklingPath.FastTypeTag[$tpe] = $createTagTree

diff --git a/core/src/main/scala/scala/pickling/internal/DefaultPicklerRegistry.scala b/core/src/main/scala/scala/pickling/internal/DefaultPicklerRegistry.scala
@@ -52,6 +52,12 @@ final class DefaultPicklerRegistry(generator: RuntimePicklerGenerator) extends P
   override def registerUnpickler[T](key: String, p: Unpickler[T]): Unit =
     unpicklerMap.put(key, p)
 
+  override private[pickling] def clearRegisteredPicklerUnpicklerFor[T: FastTypeTag]: Unit = {
+    val tag = implicitly[FastTypeTag[T]]
+    picklerMap -= tag.key
+    unpicklerMap -= tag.key
+  }
+
   override val isLookupEnabled = true
 
   /** Checks the existence of a pickler ignoring the registered generators. */

diff --git a/core/src/main/scala/scala/pickling/internal/NoReflectionRuntime.scala b/core/src/main/scala/scala/pickling/internal/NoReflectionRuntime.scala
@@ -31,6 +31,7 @@ final class NoReflectionRuntime() extends PicklingRuntime {
     override def registerPicklerGenerator[T](typeConstructorKey: String, generator: (FastTypeTag[_]) => Pickler[T]): Unit = ()
     override def registerUnpickler[T](key: String, p: Unpickler[T]): Unit = ()
     override def registerPicklerUnpickler[T](key: String, p: Pickler[T] with Unpickler[T]): Unit = ()
+    override private[pickling] def clearRegisteredPicklerUnpicklerFor[T: FastTypeTag]: Unit = ()
   }
   override val refRegistry: RefRegistry = new DefaultRefRegistry()
   override val GRL: ReentrantLock = new ReentrantLock()

diff --git a/core/src/main/scala/scala/pickling/spi/PicklerRegistry.scala b/core/src/main/scala/scala/pickling/spi/PicklerRegistry.scala
@@ -98,5 +98,7 @@ trait PicklerRegistry {
     *                   this type.
     */
   def registerPicklerUnpicklerGenerator[T](typeConstructorKey: String, generator: FastTypeTag[_] => (Pickler[T] with Unpickler[T])): Unit
-  // TODO - Some kind of clean or inspect what we have?
+
+  private[pickling] def clearRegisteredPicklerUnpicklerFor[T: FastTypeTag]: Unit
+
 }
diff --git a/core/src/test/scala/pickling/run/share-binary-any.scala b/core/src/test/scala/pickling/run/share-binary-any.scala
@@ -6,6 +6,9 @@ import scala.pickling._, scala.pickling.Defaults._, binary._
 class C(val name: String, val desc: String, var c: C, val arr: Array[Int])
 
 class ShareBinaryAnyTest extends FunSuite {
+
+  import scala.pickling.internal.currentRuntime
+
   val c1 = new C("c1", "desc", null, Array(1))
   val c2 = new C("c2", "desc", c1, Array(1))
   val c3 = new C("c3", "desc", c2, Array(1))
@@ -31,6 +34,10 @@ class ShareBinaryAnyTest extends FunSuite {
   }*/
 
   test("loop-share-nothing") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[Any]
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     intercept[StackOverflowError] {
       import shareNothing._
       c1.c = c3
@@ -59,6 +66,9 @@ class ShareBinaryAnyTest extends FunSuite {
   }*/
 
   test("noloop-share-non-primitives") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     import shareNothing._
     c1.c = null
     val pickle = (c3: Any).pickle

diff --git a/core/src/test/scala/pickling/run/share-binary.scala b/core/src/test/scala/pickling/run/share-binary.scala
@@ -15,11 +15,17 @@ final class Simple(x: Int) {
 
 
 class ShareBinaryTest extends FunSuite {
+
+  import scala.pickling.internal.currentRuntime
+
   val c1 = new C("c1", "desc", null, Array(1))
   val c2 = new C("c2", "desc", c1, Array(1))
   val c3 = new C("c3", "desc", c2, Array(1))
 
   test("loop-share-nonprimitives") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     c1.c = c3
     val pickle = c1.pickle
     //val expected = "BinaryPickle([0,0,0,29,115,99,97,108,97,46,112,105,99,107,108,105,110,103,46,115,104,97,114,101,46,98,105,110,97,114,121,46,67,0,0,0,2,99,49,0,0,0,4,100,101,115,99,-5,0,0,0,2,99,51,0,0,0,4,100,101,115,99,-5,0,0,0,2,99,50,0,0,0,4,100,101,115,99,-3,0,0,0,0,0,0,0,1,1,0,0,0,0,0,0,1,1,0,0,0,0,0,0,1,1,0,0,0])"
@@ -42,6 +48,9 @@ class ShareBinaryTest extends FunSuite {
   }
 
   test("loop-share-nothing") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     intercept[StackOverflowError] {
       import shareNothing._
       c1.c = c3
@@ -50,6 +59,9 @@ class ShareBinaryTest extends FunSuite {
   }
 
   test("loop-share-everything") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     import shareEverything._
     c1.c = c3
     val pickle = c1.pickle
@@ -71,6 +83,9 @@ class ShareBinaryTest extends FunSuite {
   }
 
   test("noloop-share-non-primitives") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     import shareNothing._
     c1.c = null
     val pickle = c3.pickle

diff --git a/core/src/test/scala/pickling/run/share-json.scala b/core/src/test/scala/pickling/run/share-json.scala
@@ -6,10 +6,17 @@ import scala.pickling._, scala.pickling.Defaults._, json._
 class C(val name: String, val desc: String, var c: C, val arr: Array[Int])
 
 class ShareJsonTest extends FunSuite {
+
+  import scala.pickling.internal.currentRuntime
+
   val c1 = new C("c1", "desc", null, Array(1))
   val c2 = new C("c2", "desc", c1, Array(1))
   val c3 = new C("c3", "desc", c2, Array(1))
+
   test("loop-share-nonprimitives") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     c1.c = c3
     val pickle = c1.pickle
     assert(pickle.toString === """
@@ -56,12 +63,11 @@ class ShareJsonTest extends FunSuite {
   }
 
   test("loop-share-nothing") {
-    /*intercept[StackOverflowError] {
-      import shareNothing._
-      c1.c = c3
-      c2.pickle
-    }*/
-    // Note we've been running out of memory on this test in jenkins, which is also a legitimate success case
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
+    // Note we've been running out of memory on this test in jenkins,
+    // which is also a legitimate success case
     try {
       import shareNothing._
       c1.c = c3
@@ -74,6 +80,9 @@ class ShareJsonTest extends FunSuite {
   }
 
   test("loop-share-everything") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     import shareEverything._
     c1.c = c3
     val pickle = c1.pickle
@@ -121,6 +130,9 @@ class ShareJsonTest extends FunSuite {
   }
 
   test("noloop-share-non-primitives") {
+
+    currentRuntime.picklers.clearRegisteredPicklerUnpicklerFor[C]
+
     import shareNothing._
     c1.c = null
     val pickle = c3.pickle