After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 683866 - Leaks of EMailFormatter object
Leaks of EMailFormatter object
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
3.6.x (obsolete)
Other Linux
: Normal major
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2012-09-12 12:45 UTC by Milan Crha
Modified: 2013-09-13 01:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
debug evo patch (1.45 KB, text/plain)
2012-09-12 12:45 UTC, Milan Crha
  Details
evo patch (1.80 KB, patch)
2012-09-13 10:00 UTC, Milan Crha
committed Details | Review

Description Milan Crha 2012-09-12 12:45:16 UTC
Created attachment 224102 [details]
debug evo patch

I had running evolution under valgrind when I got a crash which was pointing me into use of uninitialized memory:
==25418== Use of uninitialised value of size 8
==25418==    at 0x734B41C: e_extension_get_extensible (e-extension.c:187)
==25418==    by 0x20AB8A9C: headers_changed_cb (e-mail-config-format-html.c:50)

thus I tried to investigate lifetime of this object. I do not know from where the uninitialized memory usage came from, because my test patch (see the attachment) shows that either the object is kept alive, or it's freed and the headers_changed_cb() is not called for already freed objects, thus it's slightly hidden, but I found out that the used objects are kept alive forever, at least around EMailBrowser, where it's easy to test, because I expect, when I close it, then its extensions are freed as well. Try these steps:
a) apply attached patch and compile changed sources
b) run evolution
c) double click a message, to open it in an EMailBrowser;
   see on console how items are created and destroyed
d) close the EMailBrowser window
e) repeat c) and d) couple times
f) go to Edit->Preferences->Mail Preferences->Headers and tick/untick
   Reply-To header, or any other from the list;
   the console is full of headers_changed_cb() lines
g) repeat c) and d) few more times
h) repeat f) and see that there are more callbacks written

The number of prints may not increase after c) and d), basically there should be the same number of prints before c) and after d).
Comment 1 Matthew Barnes 2012-09-12 13:18:45 UTC
The signal connection made in mail_config_format_html_constructed() is never getting disconnected.  You could probably just use g_signal_connect_object() there so the connection is severed when the extension object is finalized.
Comment 2 Milan Crha 2012-09-12 17:00:58 UTC
I said the extension is not finalized when used in EMailBrowser. Check the debug patch, and follow the steps to reproduce, then you'll see yourself. The signal disconnect might not be needed, if the object is gone together with the GSettings object, where the code suggests it's intended.
Comment 3 Milan Crha 2012-09-13 10:00:38 UTC
Created attachment 224206 [details] [review]
evo patch

for evolution;

Found it, the EMailBrowser was freed as expected, but the EMailFormatter (which the EMailFormatConfigHTML extends) was not, thus there were left flying pointers. Fixing the leak makes the extension freed properly as well. I found two such leaks, one I fixed with unref, the other with using shell settings directly, to not overuse EMailFormatter.
Comment 4 Milan Crha 2012-09-13 10:02:53 UTC
Created commit 309254e in evo master (3.5.92+)