Skip to content
This repository has been archived by the owner on Feb 20, 2019. It is now read-only.

Incorrect handling of Option[xxx] if None #453

Open
jcrowley66 opened this issue May 31, 2017 · 13 comments
Open

Incorrect handling of Option[xxx] if None #453

jcrowley66 opened this issue May 31, 2017 · 13 comments

Comments

@jcrowley66
Copy link

Apologies if this is a duplicate, but several searches did not turn up this problem. [Using JSON format]

With this definition:
case class PickleNone(test:Option[String])

this sequence works OK:
val test = PickleNone(None)
val roundTrip = test.pickle.value.unpickle[PickleNone]

gives test == roundTrip

but this sequence:
val test = PickleNone(None)
val roundTrip = test.pickle.value.unpickle[AnyRef]

results in test != roundTrip

It appears in the second case that the unpickle step sees the scala.None.type and instantiates a new instance for None. Unfortunately, Scala expects None to be a singleton, and any None == None test is reduced to object identity, which now fails.

Scala 2.11.8 Pickling 0.10.1

Full test case follows

package pickletests

import scala.pickling._
import scala.pickling.Defaults._
import scala.pickling.json._

case class PickleNone(testOption:Option[String])

object PickleNoneTest extends App {
val test = PickleNone(None)
val testPickled = test.pickle.value
val testUnpickled = testPickled.unpickle[PickleNone]
val testAnyRef = testPickled.unpickle[AnyRef]
val testAnyRefAsPickleNone = testPickled.unpickle[AnyRef].asInstanceOf[PickleNone]

Console.println("test == testUnpickled is: " + (test == testUnpickled))
Console.println(" test == testAnyRef is: " + (test == testAnyRef))
Console.println(" test == asInstanceOf is: " + (test == testAnyRefAsPickleNone))

Console.println("")
Console.println("")
Console.println(" test.testOption is: " + test.testOption.toString + " @%8X".format(System.identityHashCode(test.testOption)))
Console.println(" testUnpickled.testOption is: " + testUnpickled.testOption.toString + " @%8X".format(System.identityHashCode(testUnpickled.testOption)))
Console.println(" testAnyRef.testOption is: " + testAnyRef.asInstanceOf[PickleNone].testOption.toString + " @%8X".format(System.identityHashCode(testAnyRef.asInstanceOf[PickleNone].testOption)))
Console.println("testAnyRefAsPickleNone.testOption is: " + testAnyRefAsPickleNone.testOption.toString + " @%8X".format(System.identityHashCode(testAnyRefAsPickleNone.testOption)))
}

@michielboekhoff
Copy link

michielboekhoff commented Jun 5, 2017

Just tested this, this seems to have been fixed in 0.11.x. Can you confirm this, @jcrowley66?

@jcrowley66
Copy link
Author

Ran under both 0.10.1 and 0.11.0-M2. JDK 1.8.0_121 in both cases. Both failed, and 0.11.0-M2 seems to have regressed, since each of the None instances now has a separate object ID. (Sorry about the formatting - can't quite figure it out!)

"C:\Program Files\Java\jdk1.8.0_121\bin\java"

Using scala pickling 0.10.1

test == testUnpickled is: true
test == testAnyRef is: false
test == asInstanceOf is: false

                  test.testOption is: None @4940809C
         testUnpickled.testOption is: None @4940809C
            testAnyRef.testOption is: None @7A138FC5
testAnyRefAsPickleNone.testOption is: None @379AB47B

Process finished with exit code 0

Using scala pickling 0.11.0-M2

test == testUnpickled is: false
test == testAnyRef is: false
test == asInstanceOf is: false

                  test.testOption is: None @6928F576
         testUnpickled.testOption is: None @ 9CD25FF
            testAnyRef.testOption is: None @27E0F2F5
testAnyRefAsPickleNone.testOption is: None @3574E198

Process finished with exit code 0

@jcrowley66
Copy link
Author

Sorry re the Close - hit the wrong button. Both of the previous tests run under Windows 7.

@jcrowley66 jcrowley66 reopened this Jun 5, 2017
@jcrowley66
Copy link
Author

Set up a completely clean project (using IntelliJ) with PickleNoneTest as the only source code present. Ran with 0.10.1 as the only Jar included, then deleted that and ran with 0.11.0-M2 as the only Jar. Same results.

@michielboekhoff
Copy link

I'll have a look at this later today.

@michielboekhoff
Copy link

I've been able to replicate this issue. Interesting that I hadn't been able to replicate this. I'll have a look to see if I can find a fix.

@jcrowley66
Copy link
Author

jcrowley66 commented Jun 7, 2017 via email

@michielboekhoff
Copy link

Right, so this issue is intermittently showing. I can't figure out exactly what it is. Anyone available to provide some assistance?

@michielboekhoff
Copy link

Right, I've done some more investigation and I'm going to share my findings:

Sometimes, the test I've made passes, seemingly randomly. I think what's happening is that a generated pickler for None is creating a new instance, which gets assigned a different ID. As this is not intended, I would think, it's best to write a manual pickler for Option, which I can have a go at tomorrow.

@jcrowley66
Copy link
Author

Agree, when scala.None.type is detected while unpickling it needs to be handled separately so that the unique scala.None instance is returned. Sorry that I have no experience with the innards of the pickling library, so cannot offer any advice on the implementation.

@michielboekhoff
Copy link

I don't think that'd be too hard, I'll set it up. Just wondering why the test passes intermittently... maybe force either runtime pickling or static pickling, see if that makes it better?

@jcrowley66
Copy link
Author

Now you are definitely beyond my expertise!

@jcrowley66
Copy link
Author

jcrowley66 commented Jun 13, 2017

This might be a hint - slightly different problem, but in the same ballpark. This is 0.10.1 - will try to run with 0.11.x tomorrow & report if any difference.

This source: responseTo: Option[UUIDCarrier] = None, // None if this is original message
gets pickled as:
"responseTo": {
"$type": "scala.None$"
},

which then throws an exception while trying to unpickle:

scala.pickling.PicklingException: Tag scala.None$ not recognized, looking for one of: scala.None.type, scala.Some[realtime.edt.common.UUIDCarrier]

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants