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

        Hide
        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
        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
        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
        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
        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
        Tom White added a comment -

        I just committed this.

        Show
        Tom White added a comment - I just committed this.
        Hide
        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