Avi-Levi / kryo

Automatically exported from code.google.com/p/kryo
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

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

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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 reported on code.google.com by boris.pa...@gmail.com on 12 Dec 2012 at 6:36

GoogleCodeExporter commented 9 years ago
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)

Original comment by boris.pa...@gmail.com on 12 Dec 2012 at 6:43

GoogleCodeExporter commented 9 years ago
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. 

Original comment by boris.pa...@gmail.com on 12 Dec 2012 at 6:59

GoogleCodeExporter commented 9 years ago
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().

Original comment by fuad.efe...@tokenizer.ca on 13 May 2013 at 10:40

GoogleCodeExporter commented 9 years ago
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());
        }
    }

Original comment by andreas....@gmail.com on 9 Jul 2013 at 8:49

GoogleCodeExporter commented 9 years ago
Shouldn't your LocaleSerializer be added to Kryo then? Or perhaps the 
kryo-serializers project: https://github.com/magro/kryo-serializers

Original comment by ecb0...@gmail.com on 23 Jul 2013 at 9:37

GoogleCodeExporter commented 9 years ago
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://github.com/magro/kryo-serializers) could be a good idea.

Original comment by romixlev on 24 Jul 2013 at 7:26

GoogleCodeExporter commented 9 years ago
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

Original comment by romixlev on 24 Jul 2013 at 7:36

GoogleCodeExporter commented 9 years ago
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.

Original comment by robertw...@gmail.com on 6 Sep 2013 at 12:56

GoogleCodeExporter commented 9 years ago
@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?

Original comment by romixlev on 6 Sep 2013 at 1:00

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
@romixlev: 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

Original comment by robertw...@gmail.com on 11 Nov 2013 at 1:15

GoogleCodeExporter commented 9 years ago
Is this issue still present?

Issues have moved to github btw: 
https://github.com/EsotericSoftware/kryo/issues/100

Original comment by martin.grotzke on 13 Nov 2013 at 11:47

GoogleCodeExporter commented 9 years ago
@robertwgil: If you're having an issue in combination with 
memcached-session-manager I'd suggest you submit an issue there: 
https://code.google.com/p/memcached-session-manager/issues/

I'm the author of memcached-session-manager btw. and I'd say that Locale should 
already be supported. If there's an issue it should be fairly easy to fix it.

Original comment by martin.grotzke on 13 Nov 2013 at 11:51