libical-glib: Memory leaks due to circular dependencies

Milan Crha mcrha at redhat.com
Thu Oct 8 03:25:59 EDT 2020


        Hi,
while running Evolution under valgrind I noticed that there are some
memory leaks and a deeper investigation led me to libical-glib and the
way Evolution (and evolution-data-server with evolution-ews) uses it.
I see two separate issues.

The first is around i_cal_time_get_timezone(), which returns so called
global memory and also sets an owner for this memory, which the base
ICalObject interprets as being owned and adds a reference on the
ICalTime object. This ICalTime object cannot be properly freed, because
the reference was added in error.

The second is around i_cal_timezone_set_component(), which also causes
circular dependency between the passed-in ICalComponent and the
ICalTimezone, by setting the owner of the component. The owner is used
to not have freed data being used by it.

While the first change is correct and straightforward, the second
changes the function behavior and is potentially dangerous, especially
for the code, which counts with the old/bad behavior. Sadly, the above
mentioned Evolution-related projects do misuse it. They workaround the
problem and cause the objects (in the second issue) to be freed
"properly", but it's wrong, because it's against the documentation and
against the expected usage of the function. 

I hesitate to change the i_cal_timezone_set_component(), due to its
potential to cause a disaster, thus I introduced a new function, called
i_cal_timezone_take_component(), which consumes the passed-in
ICalComponent on its own, and I document it as being a replacement for
the now-deprecated i_cal_timezone_set_component().

The things work properly with these two changes (and corresponding
changes in the Evolution-related projects).

Let me know if you've any objections/comments/anything.

        Thanks and bye,
        Milan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libical-glib-Set-owner-of-the-ICalObject-only-if-not.patch
Type: text/x-patch
Size: 1341 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20201008/7ff131a3/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-libical-glib-Introduce-i_cal_timezone_take_component.patch
Type: text/x-patch
Size: 5649 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20201008/7ff131a3/attachment-0001.bin>


More information about the libical-devel mailing list