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

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

        I just committed this.

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