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

serialization for java.util.Locale under java 1.7 is broke #100

Closed
ghost opened this issue Nov 11, 2013 · 4 comments
Closed

serialization for java.util.Locale under java 1.7 is broke #100

ghost opened this issue Nov 11, 2013 · 4 comments

Comments

@ghost
Copy link

ghost commented Nov 11, 2013

From [email protected] on December 12, 2012 19:36:49

What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?


What version of the Kryo are you using?


Please provide any additional information below.

Original issue: http://code.google.com/p/kryo/issues/detail?id=100

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on December 12, 2012 10:43:37

Sorry, accidentally pressed enter and no way to edit the issue

Steps to reproduce -

Under java 1.7

Locale l = Locale.getDefault();
Output output = new Output(MIN_BUFFER_SIZE, MAX_BUFFER_SIZE);
kryo.writeClassAndObject(output, data);

...

Locale l = (Locale)kryo.readClassAndObject(new Input(Base64.decodeBase64(sResult.getBytes("UTF-8"))));

System.out.println("After: " + l);

Throws NPE in

Exception in thread "main" java.lang.NullPointerException
at java.util.Locale.toString(Locale.java:1198)
at java.lang.String.valueOf(String.java:2902)
at java.lang.StringBuilder.append(StringBuilder.java:128)

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on December 12, 2012 10:59:15

Java 1.7 has a field called baseLocale in Locale class. This type lives under sun.* packages and is marked as transient. So there are custom read/write methods on Locale to handle serialization. But kryo does not use those.

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on May 13, 2013 15:40:13

Confirmed, root exception during deserialization:

Caused by: java.lang.NullPointerException
at java.util.Locale.getLanguage(Locale.java:1007)
at eu.livotov.tpt.i18n.Dictionary.get(Dictionary.java:83)
at eu.livotov.tpt.i18n.Dictionary.get(Dictionary.java:69)
at eu.livotov.tpt.i18n.TM.get(TM.java:63)

and line 1007:
return baseLocale.getLanguage();

Locale class has explicit readObject() and writeObject().

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on July 09, 2013 13:49:41

This Bug can be fixed by including the following line in Kryo.java below line 155:

addDefaultSerializer(Locale.class, LocaleSerializer.class);

and by adding the following lines to DefaultSerializers.java

static public class LocaleSerializer extends Serializer<Locale> {
    public void write (Kryo kryo, Output output, Locale object) {
        output.writeAscii(object.getLanguage());
    output.writeAscii(object.getCountry());
    output.writeAscii(object.getVariant());
    }

    public Locale read (Kryo kryo, Input input, Class<Locale> type) {
        return new Locale(input.readString(), input.readString(), input.readString());
    }
}

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on July 23, 2013 14:37:51

Shouldn't your LocaleSerializer be added to Kryo then? Or perhaps the kryo-serializers project: https:/magro/kryo-serializers

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From romixlev on July 24, 2013 00:26:07

I tend to agree with the previous commnent. It is not really a bug.
By default, Kryo does not cover all possible special cases present in different versions of JDKs from different vendors. It only tries to cover a common set of classes in a most generic way.

The workaround that you presented is a rather easy and usual way to add support for serialization of specific classes in user-defined code. There is no need to extend Kryo. You could simply call addDefaultSerializer right after you constructed your Kryo instances.

Adding your Locale serializer to the kryo-serializers library (https:/magro/kryo-serializers) could be a good idea.

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From romixlev on July 24, 2013 00:36:29

BTW, for any class that implements Java java.io.Serializable interface, you could use Kryo's com.esotericsoftware.kryo.serializers.JavaSerializer. It is designed exactly for such cases. It is not the fastest, but it will use read/write methods provided for java.io.Serializable.

So, you could simply use this (or any other way of registering a serializer in Kryo):
addDefaultSerializer(Locale.class, JavaSerializer.class);

-Leo

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on September 06, 2013 05:56:06

Hi, the proposed solution becomes more difficult when we are using SessionManager Kryo as tomcat using memcached for example, we are required to create a jar with the class changed.

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From romixlev on September 06, 2013 06:00:18

@-robertwgil: Can you elaborate a bit? Which of the proposed solutions you mean (there are a few in this thread)? How do you create Kryo instances in your environment? Do you create it explicitly in your code or do you use a library (e.g. library implementing your SessionManager) that internally creates Kryo instances?

@ghost
Copy link
Author

ghost commented Nov 11, 2013

From [email protected] on November 11, 2013 05:15:08

@romix: I followed this tutorial and get the problem: https://code.google.com/p/memcached-session-manager/wiki/SetupAndConfiguration
As @fuad.efendi said, the NPE occurs because baseLocale was null in java 7 when deserializating

@ghost ghost assigned magro Nov 13, 2013
@magro
Copy link
Collaborator

magro commented Nov 13, 2013

@NathanSweet Should we add a LocaleSerializer to kryo, or keep/add it to kryo-serializers?

@NathanSweet
Copy link
Member

@magro I think any JDK class that we can serialize without being JVM specific can go in Kryo's default serializers. If the serializer has to access private API via reflection, IMO it shouldn't be a default serializer. I'd be open to including ALL the kryo-serializers serializers in Kryo as long as they are in a separate package that makes it clear they are JVM specific and they are not registered by default.

@magro
Copy link
Collaborator

magro commented Nov 14, 2013

Ok, great, I submitted an issue to move serializers for std jdk classes from kryo-serializers to kryo. I also like the idea to move all serializers from kryo-serializers to kryo.

magro added a commit to magro/kryo that referenced this issue Dec 4, 2013
For deserialization by default (and if available) the static
`Locale.getInstance(String, String, String)` is used via reflection
to make use of the Locale cache. If Locale.getInstance is not available
it just falls back to the constructor.

If the `Locale(String, String, String)` constructor should be used in any case
the `LocaleSerializer(boolean)` has to be used with `useReflection`
set to `false`.
@magro
Copy link
Collaborator

magro commented Jan 25, 2014

Fixed by pull request #164.

@magro magro closed this as completed Jan 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants