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 628655 - imapx parser thread gerror memory leak
imapx parser thread gerror memory leak
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Mailer
2.32.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Milan Crha
Evolution QA team
evolution[imapx]
Depends on:
Blocks: 627707
 
 
Reported: 2010-09-03 00:40 UTC by David Woodhouse
Modified: 2013-09-14 16:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed eds patch (953 bytes, patch)
2010-09-16 12:47 UTC, Milan Crha
committed Details | Review

Description David Woodhouse 2010-09-03 00:40:49 UTC
==21464== 136 (32 direct, 104 indirect) bytes in 2 blocks are definitely lost in loss record 25,733 of 33,527
==21464==    at 0x4A0615D: malloc (vg_replace_malloc.c:195)
==21464==    by 0x3D3D645C80: g_malloc (gmem.c:134)
==21464==    by 0x3D3D65C796: g_slice_alloc (gslice.c:836)
==21464==    by 0x3D3D62C0B1: g_error_new_valist (gerror.c:54)
==21464==    by 0x3D3D62C3FD: g_set_error (gerror.c:218)
==21464==    by 0x1A26614B: imapx_command_sync_changes_done (camel-imapx-server.c:4382)
==21464==    by 0x1A26AF87: imapx_step (camel-imapx-server.c:1904)
==21464==    by 0x1A26C1EA: parse_contents (camel-imapx-server.c:4578)
==21464==    by 0x1A26C61A: imapx_parser_thread (camel-imapx-server.c:4645)
==21464==    by 0x3D3D666745: g_thread_create_proxy (gthread.c:1897)
==21464==    by 0x359B007760: start_thread (pthread_create.c:301)
==21464==    by 0x359A8E14EC: clone (clone.S:115)

This'll be the error path when we try to set flags on the server that it *told* us it didn't support. I filed a bug for that once, iirc.

We're leaking the error.
Comment 1 Milan Crha 2010-09-07 07:18:13 UTC
Does it claim on a GError overwrite in this case for you? I do not see it myself, thus wondering whether some g_propagate_error would catch this?
Comment 2 David Woodhouse 2010-09-08 15:28:15 UTC
(evolution:21464): GLib-WARNING **: GError set over the top of a previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is NULL before it's set.
The overwriting error message was: Error syncing changes: Command Argument Error. 11


You can reproduce by using an IMAP server which doesn't advertise \* in PERMANENTFLAGS and then gives you an error when your IMAP client ignores that and tries to set arbitrary flags anyway (this is bug 621837)
Comment 3 Milan Crha 2010-09-09 07:41:58 UTC
I do not have my own IMAP server, neither with such setup.
Do you have any with a testing account available, please?
(Alternatively I can try to provide a blind fix for testing.) :)
Comment 4 David Woodhouse 2010-09-09 09:18:28 UTC
I don't have *control* of any IMAP servers which are that crappy -- this is my company Exchange server. Arguably I should just fix this one myself... if I had sufficient rights in Bugzilla I'd assign it to myself.
Comment 5 Milan Crha 2010-09-16 12:47:44 UTC
Created attachment 170414 [details] [review]
proposed eds patch

for evolution-data-server;

Ah, good, I was able to reproduce it too finally, as I found a similar server.

The investigation showed that imapx is storing flags per on and off state, and for each message is created a command, but all of them under one job. The first failing command doesn't stop subsequent commands, thus all of them are calling imapx_command_sync_changes_done, but with the job's error already set.

Thus the above patch seems pretty lame, but it's how it is, just keep the first error and ignore any other.
Comment 6 Milan Crha 2010-09-29 09:50:25 UTC
Created commit 532e2cd in eds master (2.33.1+)
Created commit f95b430 in eds gnome-2-32 (2.32.1+)