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
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
