GNOME Bugzilla – Bug 515744
Evolution (IMAP) : Memory leak
Last modified: 2013-09-13 00:59:47 UTC
Evolution 2.21.91 Evolution with IMAP back end is eating residential memory.After performing some operation , residential memory consumption reached up to 308 MB and kept there. Below is the list of operation i did during my observation • receiving mails • moved to calendar • auto completion • recurrence meeting • change calendar view • move to contact • create a contact • move to task • create a task • create a task list • move to memo • create a memo - 268 MB • back to mailer • send a mail • scroll message list • mark as junk • make as not junk But i could clearly see increase in memory consumption during message list scroll and component switch.
Created attachment 104922 [details] Valgind traces of evolution process Attached are the valgrind traces which i collected during my observation.
I'm not sure what to do with this: 254 (40 direct, 214 indirect) bytes in 2 blocks are definitely lost in loss record 135 of 318 I see in code the FIXME this leaks, but I have no idea why is it there (e-cal-component.c) in such a way. I also noticed it uses attachment->attach once as a icalproperty, once as an icalattach. Very strange to me. Let Chen tell what he thinks. About my change in evo/mail/mail-component.c:impl_finalize, it isn't called anyway, because of mail_component_peek... I would like to suggest some kind of centralized "static" object owner, in e-shell or somewhere, but that's other question, really. (btw, when I manage to call it, then evo crashes on quit for me (or did in time I was working on "disable imap account" bug)). I added new bug #516080 for gtk possible leaks (it's still reachable, so possible). The pity thing is that many other leaks are from pango/cairo/X/regex which is partially because of evo static structs in the code, partially absolutely out of evo anyway. BTW, Akhill, did it crashed or not for you? I guess it did.
Well, a "leak" is defined as _repeatedly_ allocating a resource and not properly releasing it, such that over time it could potentially drain that resource. Like a leaky bucket: eventually all the water will drain out. I don't consider bounded, one-time memory allocations for such things as static structures or strings to be leaks. They're usually designed to survive until the process terminates and the kernel releases those resources. i.e. This is a leak: void foo() { gchar *duped_string = g_strdup ("some-string"); } This is not a leak: void foo() { static gchar *str_constant = NULL; if (str_constant == NULL) str_constant = g_strdup ("some-string"); }
sure, fortunately only some of them are from static members, thus should be fine, but most of them are real leaks, out of evo code probably (from my point of view).
Created attachment 105096 [details] [review] proposed evo patch for evolution; here's a patch for leaks I understood from Akhil's valgrind log. Akhil, if you can, please try with this patch, it's possible I overlooked some usages with the memory I added free call on, so it will claim with double free or something similar in that case (I touched some of them already and reverted my changes.)
Milan, after applying your patch i got memory corruption during free busy and crash during appointment creation , i think both are referring to same problem. Free-busy memory corruption - gdb traces of evolution process [New Thread 0xb22ffb90 (LWP 16487)] *** glibc detected *** /home/build/opt/gnome2/bin/evolution: free(): invalid pointer: 0x08b05cb0 *** Program received signal SIGINT, Interrupt. [Switching to Thread 0xb6699a80 (LWP 16368)] 0xffffe410 in __kernel_vsyscall () (gdb) thread apply all bt
+ Trace 189048
Thread 36 (Thread 0xb22ffb90 (LWP 16487))
Thread 1 (Thread 0xb6699a80 (LWP 16368))
Crash in evolution Gdb traces Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0xb666ea80 (LWP 17004)] icaltimezone_ensure_coverage (zone=0x4a3d, end_year=2008) at icaltimezone.c:465 465 if (!zone->component) (gdb) thread apply all bt
+ Trace 189049
Thread 1 (Thread 0xb666ea80 (LWP 17004))
Created attachment 105154 [details] [review] proposed evo patch ][ for evolution; this has removed offending part in recurrence-page.c, it really returns time zones from the local store, not new one.
Created attachment 105176 [details] [review] proposed eex patch for evolution-exchange; Here are a few bits to let eex leak a bit less than before.
(In reply to comment #8) > Created an attachment (id=105154) [edit] > proposed evo patch ][ > > for evolution; > > this has removed offending part in recurrence-page.c, it really returns time > zones from the local store, not new one. > I'm not sure of this. (2 places comp-editor-factory and e-calendar-table.c ?) tedit = COMP_EDITOR (task_editor_new (client, flags)); comp_editor_edit_comp (tedit, comp); + g_object_unref (comp); Otherwise looks fine. Push it fast. This we can check with chen and commit.
(In reply to comment #9) > Created an attachment (id=105176) [edit] > proposed eex patch > > for evolution-exchange; > > Here are a few bits to let eex leak a bit less than before. > Looks fine to commit
(In reply to comment #10) > tedit = COMP_EDITOR (task_editor_new (client, flags)); > comp_editor_edit_comp (tedit, comp); > + g_object_unref (comp); > This is correct, int comp_editor.c: real_edit_comp it clones the component. And for example same as in obj_modified_cb in same source, the comp is unreffed there too. Or in e-calendar-view.c: open_event_with_flags.
Evo part committed to trunk. Committed revision 35044. EEx part committed to trunk. Committed revision 1575.
Reopening. Milan, the e-msg-composer.c:drop_action() hunk causes a "memory freed twice" crash when dragging files into the attachment bar of a composer window. I like that you're using g_strfreev(urls) but you need to remove the two g_free(str) calls above it ('str' is an element of 'urls'). This line is somewhat misleading, and probably what caused the confusion: str = g_strstrip (urls[i]); The resulting string is not a new string. It's equivalent to: g_strstrip (urls[i]); str = urls[i]; Marking this as a blocker so we don't forget about it.
Same goes for the calendar/gui/dialogs/comp-editor.c hunk. Crash dragging an attachment into a new appointment.
Nice catch. I didnt realise g_strstrip modifies it inline.
Created attachment 105638 [details] [review] proposed evo patch ]I[ for evolution; I checked in code and these are hopefully the only places where I did it wrong with g_strsplit.
Seems fine Milan.
Committed to trunk. Committed revision 35063.
(In reply to comment #18) > Seems fine Milan. I second that; looks correct now. Thanks Milan.