chill icon indicating copy to clipboard operation
chill copied to clipboard

None not serialized properly

Open msellinger opened this issue 9 years ago • 3 comments

We are using the twitter/chill package 2.11 from Maven to add serializers for Scala objects since we are using memcached-session-manager (MSM) and our JSF beans are written in Scala. We are using new AllScalaRegistrar().apply(kryo) to register all Scala converters.

With the aforementioned setup we have the problem that if "None" values are serialized to the session and de-serialized, all sorts of strange problems happen. For example, a clause like "if (value == None)" evaluates to false even though value is actually None. The problem seems to be that None is not serialized as a singleton here, so a "different None" is created when the deserializing is done.

We were able to fix this by adding a special serializer class to our Kryo instances:

class NoneSerializer extends Serializer[None.type] {
  override def write(kryo: Kryo, output: Output, `object`: None.type): Unit = ()
  override def read(kryo: Kryo, input: Input, `type`: Class[None.type]): None.type = None
}
...
kryo.register(Class.forName("scala.None$"), new NoneSerializer())

(with credits to https://gist.github.com/Gekkio/1197273)

In this issue I see the comment

None is not needed as it is supported by the singleton serializer.

However, this does not seem to be true (or it is, but the mentioned "singleton serializer" must be registered explicitely for the "None" type).

It is possible of course that our issue only exists when using memcached-session-manager (because of different class-loaders used or whatever), but still the above fix corrects our issue so it might be worth including it into the project for others to benefit (since it took us quite some time to find this problem).

msellinger avatar Aug 03 '16 08:08 msellinger

That is a bit tricky. you need to also have a KryoInstantiator that gives you the correct default serializer (or set it up yourself). We do it here:

https://github.com/twitter/chill/blob/develop/chill-scala/src/main/scala/com/twitter/chill/KryoBase.scala#L46

there is certainly a cleaner way to do this now. We'd be happy to take pull requests, but all scala object classes will have the issue you mention. We do our best to detect that in KryoBase and do the sane thing. Your solution works for None but not others.

johnynek avatar Oct 11 '16 01:10 johnynek

So are you saying your current code already does this? What do we need to call to activate this logic? Or does it recognize Java singletons but not Scala singletons yet?

I know that a general approach would be preferred but my rationale was that if you register serializers for e.g. Some() you should also register one for None in AllScalaRegistrars. Otherwise it gives the impression that all is taken care of while actually there are subtle problems. In our code at least, manually registering singleton serializers for None, Nil and scala.xml.Null has helped a lot to fix our serialization issues.

msellinger avatar Oct 11 '16 19:10 msellinger

Yes, the code works, but you have to make a Kryo instance that is a subclass of chill's KryoBase.

https://github.com/twitter/chill/blob/develop/chill-scala/src/main/scala/com/twitter/chill/ScalaKryoInstantiator.scala#L55

These is a KryoInstantiator that does that. How are you creating your initial Kryo?

johnynek avatar Oct 11 '16 19:10 johnynek