Created attachment 107470 [details]
Partial valgrind log for evolution-exchange-storage

Here's the memory leak portion of a valgrind log showing data within EFolder instances leaking in an evolution-exchange-storage process.

The log was generated from a 1 hour run of Evolution 2.8, but I confirmed the EFolder leaks still exist on Evolution 2.22.  I'm trying to get logs for longer runs (day+) that show more severe memory consumption over time.

Matt, I have also filed a bug 517818 about memory leak in exchange connector during 2.21.x. 

Thanks Akhil.

Created attachment 107630 [details]
Partial valgrind log for evolution-exchange-storage

Here's another memcheck log from the same user, this time for a 3 hour run.

It shows EFolder objects consuming over 60MB of memory at exit, up from 20MB in the previous log.

I think I'm on to it.  Each memcheck log shows the unfreed EFolder allocations originating from exchange_hierarchy_webdav_parse_folder().

I think I found a problem in scan_subtree() (in exchange-hierarchy-webdav.c), which calls the "parse_folder" function that creates the leaking EFolders.


Relevant ChangeLog entry:

2006-02-25  Sushma Rai  <rsushma@novell.com>

        * storage/exchange-hierarchy-webdav.c (scan_subtree): Do not unref the
        folder which is being used later in subtrees.

Revision:
http://svn.gnome.org/viewvc/evolution-data-server/trunk/servers/exchange/storage/exchange-hierarchy-webdav.c?view=diff&r1=6811&r2=6812


That change was correct but it caused a side-effect, because now the folder /never/ gets unreffed.  And in a production environment you'll allocate a crapload of these instances in a single scan.

That explains why the leak is growing over time.

Created attachment 107642 [details] [review]
Proposed patch

I think this should do it.  Seems stable and valgrind confirms the leak is plugged.  Need someone more familiar with evolution-exchange to confirm it.

I can confirm that your patch will make sure to unref all the instances sanely.

Committed to trunk (revision 8579) and gnome-2-22 (revision 8580).

Reopening.  The above patch is correct, but Milan and I believe it may have uncovered a second, more serious reference counting error.

Index: storage/exchange-storage.c
===================================================================
--- storage/exchange-storage.c  (revision 1619)
+++ storage/exchange-storage.c  (working copy)
@@ -129,7 +129,7 @@ account_new_folder (ExchangeAccount *acc
 {
        const char *path = e_folder_exchange_get_path (folder);

-       e_storage_new_folder (storage, path, folder);
+       e_storage_new_folder (storage, path, g_object_ref (folder));
        if (e_folder_exchange_get_has_subfolders (folder)) {
                e_storage_declare_has_subfolders (storage, path,
                                                  _("Searching..."));

I could use a third pair of eyes on this before we ship 2.23.1.  Setting severity to BLOCKER.

Created attachment 109495 [details] [review]
Followup patch

Same as above.

Bharath, can you review it for GNOME 2.22.x/2.24

Sorry was swamped with many things. Patch looks fine. Tested and works good. Please commit.

Committed to trunk (revision 1658) and gnome-2-22 (revision 1657). Thanks for the fix Matthew.