gaob13 / kryo

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

final fields should be handled by FieldSerializer if setFieldsAsAccessible is set to true #15

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The FieldSerializer right now does not handle final fields. It should do so if 
setFieldsAsAccessible is set to true 
(default). As an alternative, it would be nice to have the possible to have 
s.th. like 
setSupportFinalFields(boolean).

Attached is a patch for FieldSerializer that checks setFieldsAsAccessible and 
does not skip the field if 
setFieldsAsAccessible is true.

Original issue reported on code.google.com by martin.grotzke on 5 Apr 2010 at 9:10

Attachments:

GoogleCodeExporter commented 9 years ago
The value of a final field can be inlined in method bodies. Changing the value 
of a
final field is possible through reflection, but should never be done because the
changes may not be seen by all code accessing the field.

Original comment by nathan.s...@gmail.com on 5 Apr 2010 at 9:15

GoogleCodeExporter commented 9 years ago
Ok, that's true. But in my case the final field is only set when an object is 
deserialized in another jvm, therefore there has not 
been any code that has seen this value.

Original comment by martin.grotzke on 5 Apr 2010 at 9:19

GoogleCodeExporter commented 9 years ago
In either case, it would be good to have the possibility to enforce this 
behavior. Do you see any problems with this?

Original comment by martin.grotzke on 5 Apr 2010 at 9:21

GoogleCodeExporter commented 9 years ago
It appears JLS 17.5.3 details when it is OK to modify final fields and the 
associated
problems:
http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.5.3
Based on this, I now think Kryo should serialize final fields. I don't think a
setting is necessary.

Original comment by nathan.s...@gmail.com on 5 Apr 2010 at 9:39

GoogleCodeExporter commented 9 years ago
Great!

Though, I just noticed that inner classes are not deserialized correctly with 
this setting. E.g.  for a class

    public static class Container {
        private final Body _body;
        public Container() {
            _body = new Body();
        }
        class Body {
        }
    }

the enclosing Container instance is not set on the Body instance after 
deserialization.
The test and serialization/deserialization looks like this:

    public void testInnerClass() throws Exception {
        final Container container = new Container();
        final Container deserialized = deserialize( serialize( container ), Container.class );
        assertDeepEquals( deserialized, container );
    }

    protected byte[] serialize( final Object o ) {
        return new ObjectBuffer(_kryo).writeObject( o );

    }

    protected <T> T deserialize( final byte[] in, final Class<T> clazz ) {
        return new ObjectBuffer( _kryo ).readObject( in, clazz );
    }

Original comment by martin.grotzke on 5 Apr 2010 at 9:49

GoogleCodeExporter commented 9 years ago
Final field support is checked in, r96.

Being unable to instantiate a non-static member class is separate from final 
field
support. To do this, Kryo would need to pass the reference to the enclosing 
instance
to Constructor#newInstance(). There is currently no mechanism to obtain the 
enclosing
instance, and it doesn't appear easy to add.

Original comment by nathan.s...@gmail.com on 6 Apr 2010 at 8:12

GoogleCodeExporter commented 9 years ago
Issue 6 has been merged into this issue.

Original comment by nathan.s...@gmail.com on 7 Apr 2010 at 2:47