avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vivek Nadkarni (Updated) (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (AVRO-930) Memory leak in resolved-writer.c in Avro C
Date Sun, 16 Oct 2011 00:02:12 GMT

     [ https://issues.apache.org/jira/browse/AVRO-930?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Vivek Nadkarni updated AVRO-930:
--------------------------------

    Status: Patch Available  (was: Open)

The patch I attached is ready for review. Please let me know if you need any changes. 
                
> Memory leak in resolved-writer.c in Avro C
> ------------------------------------------
>
>                 Key: AVRO-930
>                 URL: https://issues.apache.org/jira/browse/AVRO-930
>             Project: Avro
>          Issue Type: Bug
>          Components: c
>    Affects Versions: 1.6.0
>         Environment: All.
>            Reporter: Vivek Nadkarni
>            Priority: Minor
>             Fix For: 1.6.0
>
>         Attachments: AVRO-930.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> I was able to isolate a memory leak in the new schema resolver
> (resolved-writer.c) in Avro C on the avro-trunk (v1.6.0), in which the
> code attempts to access and free memory that has already been freed. 
> There is a simple one-line fix to this issue, using the reference
> counting mechanism provided in Avro, attached in the patch file.
> This patch is different from the one I originally sent to the Avro Dev
> mailing list, and I think my new patch is better.
> Longer Description:
> I created a small test program to test schema resolution in C. The
> program contains a writer schema with two integer elements and a reader
> schema with three integer elements. Since the reader schema has elements
> that the writer doesn't contain, and default values are not yet
> supported in C, the function try_record() goes into the "error" section
> (by design) and tries to clean up after itself. This cleanup process has
> a memory bug in it. 
> Specifically, if the reader schema contains two elements of the same
> type (in my case two ints), try_record() uses the same resolver for
> both elements. This common resolver is used because the function
> avro_resolved_writer_new_memoized() tests to see if the schema's of
> the two elements are the same, and if they are matched, it returns the
> resolver, and does not create a new resolver. It also does not
> increment the reference count of this existing resolver. So, multiple
> elements of the array of field_resolvers[] can contain pointers to the
> same resolver, and these reference are not accounted for in the
> reference count.
> During the cleanup process, there is a loop that checks if the pointer
> field_resolvers[i] is non-NULL, and tries to free all non-NULL
> resolvers. Since we have two (or more) elements of the
> field_resolvers[] array pointing to the same resolver, and the
> reference count of the resolver doesn't count these additional
> references, the code tries to call avro_value_iface_decref() on an
> already freed resolver.
> This can be simply fixed as follows: Increment the reference count
> each time you assign a resolver to the field_resolvers[] array, even
> when you are re-assigning the same resolver to a new element in the
> array.
> I verified this patch using valgrind.  Multiple valgrind errors were
> seen in try_record() and avro_refcount_dec() before applying the
> patch, and these errors went away after applying the patch.
> In an earlier email to the avro dev mailing list, I provided a patch
> which didn't use the reference count mechanism. Instead it checked the
> field_resolvers[] array for duplicate entries, and set the duplicates
> to NULL. This patch used more code, but had a more localized impact --
> in that it was exercised only when an error condition was
> encountered. I can provide this patch if anyone wants it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message