Uploaded image for project: 'Avro'
  1. Avro
  2. AVRO-1458

Setting char record field via reflection affects other fields

        Details

        • Type: Bug
        • Status: Closed
        • Priority: 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
            tomwhite 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
            tomwhite 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
            rdblue 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
            rdblue 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
            tomwhite 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
            tomwhite 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
            cutting Doug Cutting added a comment -

            +1 This looks good to me. Thanks!

            Show
            cutting Doug Cutting added a comment - +1 This looks good to me. Thanks!
            Hide
            Permalink
            jira-bot 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
            jira-bot 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
            tomwhite Tom White added a comment -

            I just committed this.

            Show
            tomwhite Tom White added a comment - I just committed this.
            Hide
            Permalink
            hudson 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 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:
                tomwhite Tom White
                Reporter:
                tomwhite Tom White
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development