Re: [Evolution-hackers] EWS MessageInfo leak wtf

2011-04-11 Thread Chenthill Palanisamy
On Sat, Apr 9, 2011 at 4:08 AM, David Woodhouse dw...@infradead.org wrote:
 ==5510== 41 bytes in 1 blocks are possibly lost in loss record 8,473 of 17,762
 ==5510==    at 0x4A05E46: malloc (vg_replace_malloc.c:195)
 ==5510==    by 0x3B6A048B52: g_malloc (gmem.c:164)
 ==5510==    by 0x3B6A06101D: g_strdup (gstrfuncs.c:102)
 ==5510==    by 0x1693D7B1: ews_message_info_clone (camel-ews-summary.c:75)
 ==5510==    by 0x1693E4FD: camel_ews_summary_add_message_info 
 (camel-ews-summary.c:417)
 ==5510==    by 0x16940ADD: camel_ews_utils_sync_created_items 
 (camel-ews-utils.c:973)
 ==5510==    by 0x16939819: sync_created_items (camel-ews-folder.c:660)
 ==5510==    by 0x16939B41: ews_refresh_info_sync (camel-ews-folder.c:750)
 ==5510==    by 0x4C78C99: camel_folder_refresh_info (camel-folder.c:1156)
 ==5510==    by 0x33B6E7F0F7: mail_msg_proxy (mail-mt.c:469)
 ==5510==    by 0x3B6A06BBC3: g_thread_pool_thread_proxy (gthreadpool.c:319)
 ==5510==    by 0x3B6A069445: g_thread_create_proxy (gthread.c:1897)
 ==5510==

 Can't repeat it; don't know how it happened... except of course that it
 obviously happened when SyncFolderItems returned some created items.

 The frame in ews_message_info_clone() at camel_ews_summary:75 is setting
 mi-change_key on the newly cloned MessageInfo:
 http://git.infradead.org/evolution-ews.git/blob/cd1face:/src/camel/camel-ews-summary.c#l75

 (I'm also *sure* it happened after commit 17bd5273 in which I fixed a
 consistent leak of our mi-change_key by adding a message_info_free()
 method to our class. Not only do I remember it that way, but the
 timestamp of my valgrind log file is about four hours later than that
 commit, and the *other* valgrind warnings that the -change_key leak
 caused are noticeable in their absence.)

 I'm confused by the use of camel_message_info_clone() in the first line
 of camel_ews_summary_add_message_info():
 http://git.infradead.org/evolution-ews.git/blob/cd1face:/src/camel/camel-ews-summary.c#l417
 There is *one* caller of this function, and it's the one in the above
 backtrace — camel_ews_utils_sync_create_items():
 http://git.infradead.org/evolution-ews.git/blob/cd1face:/src/camel/camel-ews-utils.c#l973

 And that *creates* a new MessageInfo of its own... which AFAICT never
 gets freed, although valgrind doesn't seem to be complaining about that,
 and I don't know why.

 Removing the clone in camel_ews_summary_add_message_info() and just
 using '(void *)info' seems to work just as well; I'm not really sure
 what's going on here. And either way, I've never been able to repeat the
 above warning.

 Chen, was there a reason for the clone? And why isn't valgrind
 complaining about it? Can anyone see something that would cause the
 above complaint that it *did* make?
The ews_summary_clone seems un-necessary there. I picked up some code from the
imapx provider and certainly did a mistake there. The const identifier
for the CamelMessageInfo in the
signature of the function and message_info_clone can be removed.

- Chenthill.

 --
 dwmw2


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


[Evolution-hackers] EWS MessageInfo leak wtf

2011-04-08 Thread David Woodhouse
==5510== 41 bytes in 1 blocks are possibly lost in loss record 8,473 of 17,762
==5510==at 0x4A05E46: malloc (vg_replace_malloc.c:195)
==5510==by 0x3B6A048B52: g_malloc (gmem.c:164)
==5510==by 0x3B6A06101D: g_strdup (gstrfuncs.c:102)
==5510==by 0x1693D7B1: ews_message_info_clone (camel-ews-summary.c:75)
==5510==by 0x1693E4FD: camel_ews_summary_add_message_info 
(camel-ews-summary.c:417)
==5510==by 0x16940ADD: camel_ews_utils_sync_created_items 
(camel-ews-utils.c:973)
==5510==by 0x16939819: sync_created_items (camel-ews-folder.c:660)
==5510==by 0x16939B41: ews_refresh_info_sync (camel-ews-folder.c:750)
==5510==by 0x4C78C99: camel_folder_refresh_info (camel-folder.c:1156)
==5510==by 0x33B6E7F0F7: mail_msg_proxy (mail-mt.c:469)
==5510==by 0x3B6A06BBC3: g_thread_pool_thread_proxy (gthreadpool.c:319)
==5510==by 0x3B6A069445: g_thread_create_proxy (gthread.c:1897)
==5510== 

Can't repeat it; don't know how it happened... except of course that it
obviously happened when SyncFolderItems returned some created items.

The frame in ews_message_info_clone() at camel_ews_summary:75 is setting
mi-change_key on the newly cloned MessageInfo:
http://git.infradead.org/evolution-ews.git/blob/cd1face:/src/camel/camel-ews-summary.c#l75

(I'm also *sure* it happened after commit 17bd5273 in which I fixed a
consistent leak of our mi-change_key by adding a message_info_free()
method to our class. Not only do I remember it that way, but the
timestamp of my valgrind log file is about four hours later than that
commit, and the *other* valgrind warnings that the -change_key leak
caused are noticeable in their absence.)

I'm confused by the use of camel_message_info_clone() in the first line
of camel_ews_summary_add_message_info():
http://git.infradead.org/evolution-ews.git/blob/cd1face:/src/camel/camel-ews-summary.c#l417
There is *one* caller of this function, and it's the one in the above
backtrace — camel_ews_utils_sync_create_items():
http://git.infradead.org/evolution-ews.git/blob/cd1face:/src/camel/camel-ews-utils.c#l973

And that *creates* a new MessageInfo of its own... which AFAICT never
gets freed, although valgrind doesn't seem to be complaining about that,
and I don't know why.

Removing the clone in camel_ews_summary_add_message_info() and just
using '(void *)info' seems to work just as well; I'm not really sure
what's going on here. And either way, I've never been able to repeat the
above warning.

Chen, was there a reason for the clone? And why isn't valgrind
complaining about it? Can anyone see something that would cause the
above complaint that it *did* make?

-- 
dwmw2

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers