Avro
  1. Avro
  2. AVRO-1458

Setting char record field via reflection affects other fields

        Details

        • Type: Bug Bug
        • Status: Closed
        • Priority: Blocker Blocker
        • Resolution: Fixed
        • Affects Version/s: 1.7.5
        • Fix Version/s: 1.7.7
        • Component/s: java
        • Labels:
          None
        • Hadoop Flags:
          Reviewed

          Description

          Calling ReflectData#setField for a char field causes a previously set field (e.g. boolean or byte) to become unset.

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            Tom White added a comment -

            I tracked this down to UnsafeCharField calling setInt() rather than setChar() on sun.misc.Unsafe. I’m not sure exactly why the other fields are unset (or changed), but it looks like Unsafe lives up to its name and is affecting neighbouring fields when the setter doesn’t match the type.

            It looks like the problem was introduced in Avro 1.7.5 in AVRO-1282. I’m marking it as a Blocker since the bug can cause data corruption.

            Here’s a test with a fix. Without the fix the test fails. I also added a test for ReflectData#getSchema for a char type, which was missing.

            Show
            Tom White added a comment - I tracked this down to UnsafeCharField calling setInt() rather than setChar() on sun.misc.Unsafe. I’m not sure exactly why the other fields are unset (or changed), but it looks like Unsafe lives up to its name and is affecting neighbouring fields when the setter doesn’t match the type. It looks like the problem was introduced in Avro 1.7.5 in AVRO-1282 . I’m marking it as a Blocker since the bug can cause data corruption. Here’s a test with a fix. Without the fix the test fails. I also added a test for ReflectData#getSchema for a char type, which was missing.
            Hide
            Permalink
            Ryan Blue added a comment -

            The patch fixes the bug for me, and catches the error correctly in the tests when the actual fix is not applied.

            Show
            Ryan Blue added a comment - The patch fixes the bug for me, and catches the error correctly in the tests when the actual fix is not applied.
            Hide
            Permalink
            Tom White added a comment -

            Thanks for verifying the fix, Ryan. I'd like to commit this soon unless there is any more feedback.

            Show
            Tom White added a comment - Thanks for verifying the fix, Ryan. I'd like to commit this soon unless there is any more feedback.
            Hide
            Permalink
            Doug Cutting added a comment -

            +1 This looks good to me. Thanks!

            Show
            Doug Cutting added a comment - +1 This looks good to me. Thanks!
            Hide
            Permalink
            ASF subversion and git services added a comment -

            Commit 1572912 from tomwhite@apache.org in branch 'avro/trunk'
            [ https://svn.apache.org/r1572912 ]

            AVRO-1458. Java: Setting char record field via reflection affects other fields

            Show
            ASF subversion and git services added a comment - Commit 1572912 from tomwhite@apache.org in branch 'avro/trunk' [ https://svn.apache.org/r1572912 ] AVRO-1458 . Java: Setting char record field via reflection affects other fields
            Hide
            Permalink
            Tom White added a comment -

            I just committed this.

            Show
            Tom White added a comment - I just committed this.
            Hide
            Permalink
            Hudson added a comment -

            FAILURE: Integrated in AvroJava #440 (See https://builds.apache.org/job/AvroJava/440/)
            AVRO-1458. Java: Setting char record field via reflection affects other fields (tomwhite: rev 1572912)

            • /avro/trunk/CHANGES.txt
            • /avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/FieldAccessUnsafe.java
            • /avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java
            Show
            Hudson added a comment - FAILURE: Integrated in AvroJava #440 (See https://builds.apache.org/job/AvroJava/440/ ) AVRO-1458 . Java: Setting char record field via reflection affects other fields (tomwhite: rev 1572912) /avro/trunk/CHANGES.txt /avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/FieldAccessUnsafe.java /avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java

              People

              • Assignee:
                Tom White
                Reporter:
                Tom White
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development