RuedigerMoeller / fast-serialization

FST: fast java serialization drop in-replacement
Apache License 2.0
1.59k stars 247 forks source link

Serializing a Stack results in an incorrect object #169

Closed atkaiser closed 7 years ago

atkaiser commented 7 years ago

If I serialize and then deserialize a java.util.Stack object, the process results in no exceptions, but the returned Stack object is not equal to the original. The items in the elementData Object[] are correct, but the Stack comes back with a different value for elementCount and capacityIncrement (It varies on how different it is, but the values usually end up as some large negative numbers). This causes an ArrayIndexOutOfBoundsException when you try to push something on the restored stack.

This seems related to issue #39 but it is different. The only reason DeepEquals doesn't fail in that test case is because the .equals() method on Vector only checks the elementData array and not the other values. As with that case adding the following line does work as a temporary fix (but I don't think this should have to be necessary):

conf.registerSerializer(Stack.class, new FSTCollectionSerializer(), true);

Here is a test case that fails for me:

package ser;

import java.util.Stack;

import org.junit.Test;
import org.nustaq.serialization.FSTConfiguration;
import org.nustaq.serialization.serializers.FSTCollectionSerializer;

import junit.framework.Assert;

public class StackIssue {

  @SuppressWarnings("unchecked")
  @Test
  public void test() {
    FSTConfiguration conf = FSTConfiguration.createDefaultConfiguration();
    // Adding the following line makes the test pass
    // conf.registerSerializer(Stack.class, new FSTCollectionSerializer(), true);

    Stack<Integer> stack = new Stack<>();

    byte[] byteArray = conf.asByteArray(stack);
    Stack<Integer> restoredStack = (Stack<Integer>) conf.asObject(byteArray);

    // The following results in a ArrayIndexOutOfBoundsException as the size of
    // the restoredStack is usually some large negative number.
    // restoredStack.push(0);

    Assert.assertTrue(stack.size() == restoredStack.size());
  }

}

I'm currently using version 2.48, let me know if there is anything I can do to help debug/fix this.

RuedigerMoeller commented 7 years ago

Thanks a lot for reporting. Will investigate asap.

atkaiser commented 7 years ago

So I tried to debug this and didn't really get that far, but here is what I discovered.

First off the actual bytes that get written and read back in seem to be correct, so I don't think there is a problem there. The problem seems to be coming from trying to set the read in values on the newly created object after it has read them in. Specifically the failing line is at FSTObjectInput.java line 625:

fstFieldInfo.setObjectValue(toRead,val);

While debugging the val variable seems to be correct, yet this method doesn't seem to set the variable correctly on the toRead object. For example when I dump the toRead object before this method I get:

main[1] dump toRead
 toRead = {
    serialVersionUID: 1224463164541339165
    java.util.Vector.elementData: null
    java.util.Vector.elementCount: 0
    java.util.Vector.capacityIncrement: 0
    java.util.Vector.serialVersionUID: -2767605614048989439
    java.util.Vector.MAX_ARRAY_SIZE: 2147483639
    java.util.AbstractList.modCount: 0
    java.util.AbstractCollection.MAX_ARRAY_SIZE: 2147483639
}

And when I dump it after this method it looks like:

main[1] dump toRead
 toRead = {
    serialVersionUID: 1224463164541339165
    java.util.Vector.elementData: null
    java.util.Vector.elementCount: 0
    java.util.Vector.capacityIncrement: -313106974
    java.util.Vector.serialVersionUID: -2767605614048989439
    java.util.Vector.MAX_ARRAY_SIZE: 2147483639
    java.util.AbstractList.modCount: 0
    java.util.AbstractCollection.MAX_ARRAY_SIZE: 2147483639
}

(The capacityIncrement is the field that is trying to be set with a value of 1) At this point I don't quite know enough about how sun.misc.Unsafe.putObject works to figure out why this is failing, but hopefully this helps.

Also one thing to note is that serializing a Vector works because in line 470 of FSTConfiguration.java there is the line reg.putSerializer(Vector.class, new FSTCollectionSerializer(), false); so a quick fix to this bug would be to just add the line reg.putSerializer(Stack.class, new FSTCollectionSerializer(), false); to that file or change that line to reg.putSerializer(Vector.class, new FSTCollectionSerializer(), true); but that doesn't really fix the underlying issue.

RuedigerMoeller commented 7 years ago

Hi, thanks a lot for your contribution. The fundamental reason is caused by the "asymetric" serialization implementation of java.util.Vector (which is flawed IMO). it implements "writeObject" like

    private void writeObject(java.io.ObjectOutputStream s)
            throws java.io.IOException {
        final java.io.ObjectOutputStream.PutField fields = s.putFields();
        final Object[] data;
        synchronized (this) {
            fields.put("capacityIncrement", capacityIncrement);
            fields.put("elementCount", elementCount);
            data = elementData.clone();
        }
        fields.put("elementData", data);
        s.writeFields();
    }

but does not implement "readObject()", so "writeObject()" makes strong assumptions about the binary format of the default implementation of "readObject()". As fst uses an optimized binary format if no custom functions are present (read/writeObject, ..), this asymetry leads to a read failure.

Some few classes of JDK (also Android SDK) construct such (imo invalid) implementation-dependencies which can be solved only by registering a custom serializer.

RuedigerMoeller commented 7 years ago

i'll follow your proposal and reuse the FSTCollectionSerializer().

RuedigerMoeller commented 7 years ago

fixed with 2.52

caglartolgatetik commented 7 years ago

Thanks a lot. It works. :)