[prev in list] [next in list] [prev in thread] [next in thread] 

List:       avro-dev
Subject:    [jira] [Updated] (AVRO-930) Memory leak in resolved-writer.c in Avro C
From:       "Douglas Creager (Updated) (JIRA)" <jira () apache ! org>
Date:       2011-10-24 14:31:32
Message-ID: 648140010.8680.1319466692146.JavaMail.tomcat () hel ! zones ! apache ! org
[Download RAW message or body]


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

Douglas Creager updated AVRO-930:
---------------------------------

    Resolution: Fixed
      Assignee: Douglas Creager
        Status: Resolved  (was: Patch Available)

Sorry for the delay in committing this —  I was away over the weekend.  This patch \
is in SVN trunk now.  Thanks Vivek!  
> 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
> Assignee: Douglas Creager
> Priority: Blocker
> Fix For: 1.6.0
> 
> Attachments: AVRO-930.patch, avro-930-test.c
> 
> 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


[prev in list] [next in list] [prev in thread] [next in thread]