Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

None not serialized properly #256

Open
msellinger opened this issue Aug 3, 2016 · 3 comments
Open

None not serialized properly #256

msellinger opened this issue Aug 3, 2016 · 3 comments

Comments

@msellinger
Copy link

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).

@johnynek
Copy link
Collaborator

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.

@msellinger
Copy link
Author

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.

@johnynek
Copy link
Collaborator

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?

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

No branches or pull requests

2 participants