Bug 3155

Summary: Memory leaks found by Valgrind in a9065aec26499a0e1294c73b6d9e6f039976521e
Product: Claws Mail (GTK 2) Reporter: Aleksander Mazur <deweloper>
Component: OtherAssignee: users
Status: RESOLVED FIXED    
Severity: normal    
Priority: P3    
Version: 3.10.0   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Memory leaks found by Valgrind in a9065aec26499a0e1294c73b6d9e6f039976521e
none
Annotated leak report none

Description Aleksander Mazur 2014-04-27 11:09:58 UTC
Created attachment 1367 [details]
Memory leaks found by Valgrind in a9065aec26499a0e1294c73b6d9e6f039976521e

First of all, thanks for the bugfixes you have already done. They really improve stability of claws-mail.
Now I would like to attach a list of memory leaks detected by Valgrind during a few hours of operation of claws-mail a9065aec26499a0e1294c73b6d9e6f039976521e.
While some of them seem to be caused solely by libraries such as Pango, still many have claws-mail source files on callstacks.
Since an e-mail client is a kind of application which is commonly being run "infinitely", in the background, IMHO it is important to eliminate the leaks in order to avoid running out of memory and terminating the application.
Comment 1 users 2014-04-28 13:08:04 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2014-04-28 13:08:03.844137679 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=20a51f4058bdb51af846ef48be05b58642378389
Merge: 84d687a e58f813
Author: Colin Leroy <colin@colino.net>
Date:   Mon Apr 28 13:08:03 2014 +0200

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=e58f813baf41ff49f104ada1240028d8c8184ede
Merge: 5454ca5 8704da3
Author: Colin Leroy <colin@colino.net>
Date:   Mon Apr 28 13:07:33 2014 +0200

    Merge branch 'master' of ssh+git://git.claws-mail.org/home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=5454ca5af43ebb23c45f6f1890705a7425008d86
Author: Colin Leroy <colin@colino.net>
Date:   Mon Apr 28 12:53:32 2014 +0200

    Remove useless cruft from OpenSSL days

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=038064957d9f51f355bd33418e84e68a127ad897
Author: Colin Leroy <colin@colino.net>
Date:   Mon Apr 28 12:53:20 2014 +0200

    Fix type

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=1e30bfc631ee26191a0b4e9edc8bde63a0d7fa99
Author: Colin Leroy <colin@colino.net>
Date:   Mon Apr 28 12:28:42 2014 +0200

    Fix most of the leaks referenced in bug #3155
Comment 2 Colin Leroy 2014-04-28 13:09:01 UTC
Created attachment 1369 [details]
Annotated leak report
Comment 3 Colin Leroy 2014-04-30 21:07:28 UTC
Closing! :)
Comment 4 Aleksander Mazur 2014-05-01 18:36:45 UTC
I've upgraded to claws-mail 8880d1a9996875e5cb872509de1d47c22e0b9b04 and still see leaks like those you previously commented with "weird. Will investigate later.":

==00:00:52:10.981 2787== 19,736 (13,020 direct, 6,716 indirect) bytes in 3 blocks are definitely lost in loss record 17,264 of 17,329
==00:00:52:10.981 2787==    at 0x40087BA: calloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==00:00:52:10.981 2787==    by 0x4806B529: g_malloc0 (gmem.c:134)
==00:00:52:10.981 2787==    by 0x80DD1C9: imap_session_get (imap.c:1180)
==00:00:52:10.981 2787==    by 0x80DED0F: imap_scan_required (imap.c:4712)
==00:00:52:10.981 2787==    by 0x80D4439: folderview_check_new (folderview.c:1145)
==00:00:52:10.981 2787==    by 0x80EDF88: inc_all_account_mail (inc.c:362)
==00:00:52:10.981 2787==    by 0x80EEB40: defer_check_all (main.c:314)
==00:00:52:10.981 2787==    by 0x48066261: g_timeout_dispatch (gmain.c:4451)
==00:00:52:10.981 2787==    by 0x48065555: g_main_context_dispatch (gmain.c:3066)
==00:00:52:10.981 2787==    by 0x4806591F: g_main_context_iterate.isra.23 (gmain.c:3713)
==00:00:52:10.981 2787==    by 0x48065DC2: g_main_loop_run (gmain.c:3907)
==00:00:52:10.981 2787==    by 0x454E66FF: gtk_main (gtkmain.c:1257)
==00:00:52:10.981 2787== 

==00:00:56:04.218 2831== 19,736 (13,020 direct, 6,716 indirect) bytes in 3 blocks are definitely lost in loss record 17,252 of 17,317
==00:00:56:04.218 2831==    at 0x40087BA: calloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==00:00:56:04.218 2831==    by 0x4806B529: g_malloc0 (gmem.c:134)
==00:00:56:04.219 2831==    by 0x80DD1C9: imap_session_get (imap.c:1180)
==00:00:56:04.219 2831==    by 0x80DED0F: imap_scan_required (imap.c:4712)
==00:00:56:04.219 2831==    by 0x80D4439: folderview_check_new (folderview.c:1145)
==00:00:56:04.219 2831==    by 0x80EDF88: inc_all_account_mail (inc.c:362)
==00:00:56:04.219 2831==    by 0x80EEB40: defer_check_all (main.c:314)
==00:00:56:04.219 2831==    by 0x48066261: g_timeout_dispatch (gmain.c:4451)
==00:00:56:04.219 2831==    by 0x48065555: g_main_context_dispatch (gmain.c:3066)
==00:00:56:04.219 2831==    by 0x4806591F: g_main_context_iterate.isra.23 (gmain.c:3713)
==00:00:56:04.219 2831==    by 0x48065DC2: g_main_loop_run (gmain.c:3907)
==00:00:56:04.219 2831==    by 0x454E66FF: gtk_main (gtkmain.c:1257)
==00:00:56:04.219 2831== 

Looking at imap_session_get() in imap.c:989 I think I can see two paths which lead to loosing a valid session pointer.

1. Let's assume that rfolder->session = NULL @1017 so session = imap_session_new() executes @1032;
the new session is obviously not-yet-authenticated so we branch to imap_session_authenticate() @1042, what fails with a fatal error.
In such case imap_safe_destroy(session) is not called but the session pointer is also not being stored anywhere.

2. If imap_cmd_noop(session) @1067 fails with a fatal error, pointer to the old session (held in rfolder->session and local variable "session") is being lost (replaced with a newly allocated one) without a call to imap_safe_destroy().

It seems that in case of fatal errors imap_handle_error() internally calls imap_disc_session_destroy(), but this one doesn't free any memory.
Comment 5 Aleksander Mazur 2014-05-01 18:51:36 UTC
The following leak, marked as "fixed", still happens:

==00:00:56:04.210 2831== 152 bytes in 2 blocks are definitely lost in loss record 15,631 of 17,317
==00:00:56:04.210 2831==    at 0x4006B11: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==00:00:56:04.210 2831==    by 0x4806B4B1: g_malloc (gmem.c:104)
==00:00:56:04.210 2831==    by 0x480845AB: g_strconcat (gstrfuncs.c:589)
==00:00:56:04.210 2831==    by 0x80E1B4E: imap_cache_msg (imap.c:1588)
==00:00:56:04.210 2831==    by 0x80E9DB9: imap_gtk_synchronise (imap_gtk.c:449)
==00:00:56:04.210 2831==    by 0x80DD9C1: imap_synchronise (imap.c:6055)
==00:00:56:04.210 2831==    by 0x80C8E80: folder_item_synchronise (folder.c:4546)
==00:00:56:04.210 2831==    by 0x80C7687: folder_item_scan_full (folder.c:2386)
==00:00:56:04.210 2831==    by 0x80C92C6: folder_item_scan (folder.c:2483)
==00:00:56:04.210 2831==    by 0x80D44E6: folderview_check_new (folderview.c:1149)
==00:00:56:04.210 2831==    by 0x80EDF88: inc_all_account_mail (inc.c:362)
==00:00:56:04.210 2831==    by 0x81BFE3E: toolbar_inc_all_cb (toolbar.c:2683)

Looks like filename is being free'd only if it describes an existing file, otherwise g_free() isn't called (?)

Valgrind has also detected a new leak (not present in previously attached report):

==00:00:56:04.210 2831== 156 bytes in 3 blocks are definitely lost in loss record 15,639 of 17,317
==00:00:56:04.210 2831==    at 0x4006B11: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==00:00:56:04.210 2831==    by 0x4806B4B1: g_malloc (gmem.c:104)
==00:00:56:04.210 2831==    by 0x480843CB: g_strndup (gstrfuncs.c:428)
==00:00:56:04.210 2831==    by 0x480463B5: strdup_len (gconvert.c:878)
==00:00:56:04.210 2831==    by 0x480475D7: g_filename_from_utf8 (gconvert.c:1233)
==00:00:56:04.210 2831==    by 0x811566D: mh_filename_from_utf8 (mh.c:1214)
==00:00:56:04.211 2831==    by 0x8115ED0: mh_item_get_path (mh.c:858)
==00:00:56:04.211 2831==    by 0x80C4A0A: folder_item_get_path (folder.c:1890)
==00:00:56:04.211 2831==    by 0x80C66D2: folder_get_default_processing (folder.c:4266)
==00:00:56:04.211 2831==    by 0x80ECB7E: inc_start (inc.c:702)
==00:00:56:04.211 2831==    by 0x80EE018: inc_all_account_mail (inc.c:388)
==00:00:56:04.211 2831==    by 0x80EE158: inc_autocheck_func (inc.c:1497)
Comment 6 users 2014-05-01 20:13:03 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2014-05-01 20:13:03.587039375 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=7aaa6ae0f1aa58f38fded118ad2d1bc6e0bb3cc8
Merge: 2f82c4d f09ae1f
Author: Colin Leroy <colin@colino.net>
Date:   Thu May 1 20:13:03 2014 +0200

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=f09ae1ff789f4225465fdef33da930fac8f00191
Author: Colin Leroy <colin@colino.net>
Date:   Thu May 1 20:11:35 2014 +0200

    More leak fix for bug #3155

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=68da8a3e5f60f04fca65e2cb3c58384d530653da
Author: Colin Leroy <colin@colino.net>
Date:   Thu May 1 18:56:21 2014 +0200

    Really fix leak (Thanks!)