Thrift
  1. Thrift
  2. THRIFT-772

__isset_bit_vector state before serialization differs from state after unserialization on native field

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 0.2
    • Fix Version/s: None
    • Component/s: Java - Library
    • Labels:
    • Environment:

      Linux, thrift release 0.2.0 with patch Thrift-746 and Thrift-663

      Description

      Once java classes have been generated it is possible to test if the fields have been set or not using the method isSetXXX().

      Once one of this java class is instanciated, all call to field.isSetXXX() are returning false which is a good thing (this method ask for __isset_bit_vector.get(XXX))

      Now if you serialize the previous object and unserialize it (without doing any changes on it) the following occured : all fields that corresponds to native data are returning true when isSetXXX() is invoked.

      Consequently, the operation unserialise(serialize(object)) is not an identity function.

      This is mainly due that the __isset_bit_vector is not serialize and rebuild during the unserialize operation.

      Any idea how to fix this ?

        Activity

        Hide
        david herviou added a comment -

        I've attached the unit test for this use case.

        Show
        david herviou added a comment - I've attached the unit test for this use case.
        Hide
        Bryan Duxbury added a comment -

        I'll glance at your unit test when I get a chance, but are your struct's fields "default" (that is, not required or optional)? If so, then no matter what the state of isset is, native fields will be serialized, and thus deserialized.

        This is a holdover from earlier times which I'm not particularly crazy about, but the good news is that you can work around it by making optional/required explicit on every field.

        Show
        Bryan Duxbury added a comment - I'll glance at your unit test when I get a chance, but are your struct's fields "default" (that is, not required or optional)? If so, then no matter what the state of isset is, native fields will be serialized, and thus deserialized. This is a holdover from earlier times which I'm not particularly crazy about, but the good news is that you can work around it by making optional/required explicit on every field.
        Hide
        david herviou added a comment -

        Indeed, putting fields 'optional' will does the trick. But actually the thing that make me a little bit embarass is the non identity of unserialize(serialize(object)) behavior for the bit set.
        If this is something assumed and not to be used this way, that's ok.

        Do you want this issue status to be changed ? or is this something you want to keep an eye on ?

        Show
        david herviou added a comment - Indeed, putting fields 'optional' will does the trick. But actually the thing that make me a little bit embarass is the non identity of unserialize(serialize(object)) behavior for the bit set. If this is something assumed and not to be used this way, that's ok. Do you want this issue status to be changed ? or is this something you want to keep an eye on ?
        Hide
        Bryan Duxbury added a comment -

        At least until we decide to get rid of the "default" requiredness, then I think we should close this issue.

        Show
        Bryan Duxbury added a comment - At least until we decide to get rid of the "default" requiredness, then I think we should close this issue.
        Hide
        Jeff Whiting added a comment -

        This bug seems like a big problem. We have two thrift objects that are the same but when you call o1.equals(o2) it returns false because the __isset_bit_vector state is different. We tried the work around of putting "optional" but it didn't do anything. We even tried "required" but it didn't change it.

        We are trying to use equals (or compareTo) in a unit test for one our thrift services. So we do:

        ThriftObj o1 = new ThriftObj();
        o1.setBooleanValue(true);

        client.sendThriftObject(o1);
        ThriftObj o2 = client.getThriftObject();

        assert( o1.equals(o2) );

        And the assert fails because the __isset_bit_vector is different even though ALL of the values are identical. Why even bother with an equals and a compareTo if it wont return true for thrift objects that have the the same values?

        Show
        Jeff Whiting added a comment - This bug seems like a big problem. We have two thrift objects that are the same but when you call o1.equals(o2) it returns false because the __isset_bit_vector state is different. We tried the work around of putting "optional" but it didn't do anything. We even tried "required" but it didn't change it. We are trying to use equals (or compareTo) in a unit test for one our thrift services. So we do: ThriftObj o1 = new ThriftObj(); o1.setBooleanValue(true); client.sendThriftObject(o1); ThriftObj o2 = client.getThriftObject(); assert( o1.equals(o2) ); And the assert fails because the __isset_bit_vector is different even though ALL of the values are identical. Why even bother with an equals and a compareTo if it wont return true for thrift objects that have the the same values?
        Hide
        Bryan Duxbury added a comment -

        I can assure you that equals and compareTo work. We use those methods all the time. I don't know what other fields your ThriftObj has, but you should make all of the fields either required or optional. This will ensure that the optional ones are not serialized or deserialized on either side, and the bit vector will be correct.

        Also, what version of Thrift are you using?

        Show
        Bryan Duxbury added a comment - I can assure you that equals and compareTo work. We use those methods all the time. I don't know what other fields your ThriftObj has, but you should make all of the fields either required or optional. This will ensure that the optional ones are not serialized or deserialized on either side, and the bit vector will be correct. Also, what version of Thrift are you using?
        Hide
        Jeff Whiting added a comment -

        Our work around for the broken equals function (which isn't very pretty):

        for (Response._Fields f : Response._Fields.values())
        {
        Object value1 = o1.getFieldValue(f);
        Object value2 = o2.getFieldValue(f);
        if (value1 instanceof byte[])

        { value1 = ByteBuffer.wrap((byte[])value1); value2 = ByteBuffer.wrap((byte[])value2); }

        if (!value1.equals(value2))

        { return false; }

        }
        return true;

        Show
        Jeff Whiting added a comment - Our work around for the broken equals function (which isn't very pretty): for (Response._Fields f : Response._Fields.values()) { Object value1 = o1.getFieldValue(f); Object value2 = o2.getFieldValue(f); if (value1 instanceof byte[]) { value1 = ByteBuffer.wrap((byte[])value1); value2 = ByteBuffer.wrap((byte[])value2); } if (!value1.equals(value2)) { return false; } } return true;

          People

          • Assignee:
            Unassigned
            Reporter:
            david herviou
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development