Avro
  1. Avro
  2. AVRO-1034

Avro C Resolved reader does not initialize children of arrays, resulting in segmentation faults

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.2
    • Fix Version/s: 1.6.3
    • Component/s: c
    • Labels:
      None
    • Environment:

      GNU/Linux Ubuntu 11.10 64-bit

      Description

      As I did in AVRO-984, I created a test program that creates an avro
      value corresponding to the following schema:

      {"type":"array", "items": {"type": "array", "items": "long"}}

      The avro_value is then resolved using a resolved_reader, and an
      attempt is made to read from the resolved_reader. This results in a
      segmentation fault on Linux.

      I believe this issue is similar to the issue in AVRO-984, in which
      nested arrays did not work because avro_resolved_array_writer_init()
      did not recursively call the init function on its children. In that
      case the initialization had to be deferred until a new item was
      appended to the array.

      I believe that avro_resolved_array_reader_init() should also
      recursively call the init function on its children, in a deferred
      manner as in AVRO-984, but I don't know how to implement this yet.

      I will attach a test program that shows the crash.

      Thanks,
      Vivek

      1. avro-1034-test-2.c
        13 kB
        Vivek Nadkarni
      2. avro-1034-test.c
        12 kB
        Vivek Nadkarni
      3. AVRO-1034.patch
        0.6 kB
        Vivek Nadkarni
      4. 0001-AVRO-1034.-C-Resolved-reader-initializes-child-array.patch
        14 kB
        Douglas Creager
      5. 0001-AVRO-1034.-C-Resolved-reader-initializes-child-array.patch
        16 kB
        Douglas Creager

        Activity

        Vivek Nadkarni created issue -
        Hide
        Vivek Nadkarni added a comment -

        This test file demonstrates a segmentation fault when trying to read a nested array from a resolved-reader. This test file can be added to the list of tests in Avro-C.

        Show
        Vivek Nadkarni added a comment - This test file demonstrates a segmentation fault when trying to read a nested array from a resolved-reader. This test file can be added to the list of tests in Avro-C.
        Vivek Nadkarni made changes -
        Field Original Value New Value
        Attachment avro-1034-test.c [ 12515536 ]
        Hide
        Vivek Nadkarni added a comment -

        The attached patch calls avro_resolved_reader_init(), on the value
        previously returned by avro_resolved_array_reader_get_by_index(), and
        returns the result of that call. This patch fixes the segmentation
        fault in the attached test case.

        However, I am not sure if this is the right way to fix the array
        initialization. I would appreciate it if someone else could review the
        patch.

        One concern with the patch is that the init function is called every
        time the function avro_resolved_array_reader_get_by_index() is
        called. So for example, the init function is called each time
        avro_value_copy() is called to copy the resolved value into another
        avro value. It is not clear to me whether such behavior is desirable.

        Thanks again,
        Vivek

        Show
        Vivek Nadkarni added a comment - The attached patch calls avro_resolved_reader_init(), on the value previously returned by avro_resolved_array_reader_get_by_index(), and returns the result of that call. This patch fixes the segmentation fault in the attached test case. However, I am not sure if this is the right way to fix the array initialization. I would appreciate it if someone else could review the patch. One concern with the patch is that the init function is called every time the function avro_resolved_array_reader_get_by_index() is called. So for example, the init function is called each time avro_value_copy() is called to copy the resolved value into another avro value. It is not clear to me whether such behavior is desirable. Thanks again, Vivek
        Vivek Nadkarni made changes -
        Attachment AVRO-1034.patch [ 12515544 ]
        Hide
        Vivek Nadkarni added a comment -

        Just posted a patch which fixes the segmentation fault in the uploaded test case.

        Show
        Vivek Nadkarni added a comment - Just posted a patch which fixes the segmentation fault in the uploaded test case.
        Vivek Nadkarni made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Douglas Creager added a comment -

        Here's a different patch. It only initializes any given element of the array when that element is created. That happens the first time that element, or any element that appears after it, is accessed. This fixes the segfault in Vivek's test case.

        If I don't hear objections, I'll commit this patch to SVN tomorrow so that it makes it in for 1.6.3.

        Show
        Douglas Creager added a comment - Here's a different patch. It only initializes any given element of the array when that element is created. That happens the first time that element, or any element that appears after it, is accessed. This fixes the segfault in Vivek's test case. If I don't hear objections, I'll commit this patch to SVN tomorrow so that it makes it in for 1.6.3.
        Douglas Creager made changes -
        Hide
        Vivek Nadkarni added a comment -

        I am attaching a new test case avro-1034-test-2.c. This test case
        results in a segmentation fault when I apply Doug's patch
        0001-AVRO-1034.-C-Resolved-reader-initializes-child-array.patch, but
        does not result in a segmentation fault when I apply my patch
        AVRO-1034.patch.

        I don't know if the problem is with Doug's patch or if I am operating
        incorrectly on the resolved value in my test case. Specifically, I am
        calling avro_value_reset() on the resolved value.

        In this test case I populate a source value (nested) with a nested
        array and read it using a resolved reader value (resolved_record). I
        then call avro_value_reset on both the source value and the resolved
        reader value. I then re-populate the source value with a larger nested
        array, and proceed to read it using the same resolved reader.

        On this second read attempt, the code seg-faults, when I am using
        Doug's patch. However, if I do not call avro_value_reset() on the
        resolved reader value, then the code does not seg-fault and appears to
        operate correctly, again using Doug's patch.

        Is it permissible to call avro_value_reset() on the resolved reader
        value?

        Thanks,
        Vivek

        Show
        Vivek Nadkarni added a comment - I am attaching a new test case avro-1034-test-2.c. This test case results in a segmentation fault when I apply Doug's patch 0001- AVRO-1034 .-C-Resolved-reader-initializes-child-array.patch, but does not result in a segmentation fault when I apply my patch AVRO-1034 .patch. I don't know if the problem is with Doug's patch or if I am operating incorrectly on the resolved value in my test case. Specifically, I am calling avro_value_reset() on the resolved value. In this test case I populate a source value (nested) with a nested array and read it using a resolved reader value (resolved_record). I then call avro_value_reset on both the source value and the resolved reader value. I then re-populate the source value with a larger nested array, and proceed to read it using the same resolved reader. On this second read attempt, the code seg-faults, when I am using Doug's patch. However, if I do not call avro_value_reset() on the resolved reader value, then the code does not seg-fault and appears to operate correctly, again using Doug's patch. Is it permissible to call avro_value_reset() on the resolved reader value? Thanks, Vivek
        Vivek Nadkarni made changes -
        Attachment avro-1034-test-2.c [ 12516526 ]
        Hide
        Douglas Creager added a comment -

        Here's an updated version of my patch that eliminates the segfault in the second test case. Just needed to add an avro_array_raw_clear call to the reset method.

        Show
        Douglas Creager added a comment - Here's an updated version of my patch that eliminates the segfault in the second test case. Just needed to add an avro_array_raw_clear call to the reset method.
        Douglas Creager made changes -
        Hide
        Douglas Creager added a comment -

        Committed this to SVN to make sure it's ready for when Doug cuts the 1.6.3 release.

        Show
        Douglas Creager added a comment - Committed this to SVN to make sure it's ready for when Doug cuts the 1.6.3 release.
        Douglas Creager made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Doug Cutting made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Unassigned
            Reporter:
            Vivek Nadkarni
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 168h
              168h
              Remaining:
              Remaining Estimate - 168h
              168h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development