Re: [Evolution-hackers] Memory leak in Camel's CamelDiscoFolder's getv

[Philip Van Hoof <spam pvanhoof be>]

On Wed, 2007-09-26 at 10:04 +0000, Srinivasa Ragavan wrote:

Hey Srinivasa,

> Have a patch to fix it?

I more or less disabled the writing of instance state in camel-lite for
now, so I don't have an actual correct fix for this yet (I'm not using
this state recovery functionality, but I don't really see how Evolution
ever uses this either. Looks to me like foobar functionality, but I
could be very wrong too).

Perhaps can Jeffrey give some view on why this feature exists in Camel? 

In any case, this leak happens during the writing of that state to disk.
The part where it gets a copy of CamelArgGetV-s which it can then write
seems to leak.

I'm not sure but I think simply not doing g_slist_copy and just using
disco_folder_properties (the first node of it) should work too, and
wouldn't cause the leak. Although I recommend reviewing such a change by
the original author (I don't know the intention of this code well
enough).


> -Srini
> On Wed, 2007-09-26 at 11:06 +0200, Philip Van Hoof wrote:
> > There's a memory leak in camel-disco-folder.c
> > 
> > static int
> > disco_getv(CamelObject *object, CamelException *ex, CamelArgGetV *args)
> > {
> > 	int i, count=0;
> > 	guint32 tag;
> > 
> > 	for (i=0;i<args->argc;i++) {
> > 		CamelArgGet *arg = &args->argv[i];
> > 
> > 		tag = arg->tag;
> > 
> > 		switch (tag & CAMEL_ARG_TAG) {
> > 		case CAMEL_OBJECT_ARG_PERSISTENT_PROPERTIES:
> > 		case CAMEL_FOLDER_ARG_PROPERTIES: {
> > 			CamelArgGetV props;
> > 
> > 			props.argc = 1;
> > 			props.argv[0] = *arg;
> > 			((CamelObjectClass *)parent_class)->getv(object, ex, &props);
> > --->			*arg->ca_ptr = g_slist_concat(*arg->ca_ptr, g_slist_copy(disco_folder_properties));
> > 			break; }
> > 			/* disco args */
> > 		case CAMEL_DISCO_FOLDER_ARG_OFFLINE_SYNC:
> > 			*arg->ca_int = ((CamelDiscoFolder *)object)->offline_sync;
> > 			break;
> > 		default:
> > 			count++;
> > 			continue;
> > 		}
> > 
> > 		arg->tag = (tag & CAMEL_ARG_TYPE) | CAMEL_ARG_IGNORE;
> > 	}
> > 
> > 	if (count)
> > 		return ((CamelObjectClass *)parent_class)->getv(object, ex, args);
> > 
> > 	return 0;
> > }
> > 
> > 
> > ==15153== 53,610 (11,989 direct, 41,621 indirect) bytes in 323 blocks are definitely lost in loss record 184 of 195
> > ==15153==    at 0x4020626: malloc (vg_replace_malloc.c:149)
> > ==15153==    by 0x4CECE9C: g_malloc (in /targets/w37/usr/lib/libglib-2.0.so.0.1200.12)
> > ==15153==    by 0x4CFD38A: g_slice_alloc (in /targets/w37/usr/lib/libglib-2.0.so.0.1200.12)
> > ==15153==    by 0x4CFDC51: g_slist_copy (in /targets/w37/usr/lib/libglib-2.0.so.0.1200.12)
> > ==15153==    by 0x4126F53: disco_getv (camel-disco-folder.c:205)
> > ==15153==    by 0x684ECFD: imap_getv (camel-imap-folder.c:761)
> > ==15153==    by 0x419E298: camel_object_get (camel-object.c:1574)
> > ==15153==    by 0x419E392: cobject_state_write (camel-object.c:517)
> > ==15153==    by 0x419EA69: camel_object_state_write (camel-object.c:1834)
> > ==15153==    by 0x4127459: disco_sync (camel-disco-folder.c:282)
> > ==15153==    by 0x413663B: camel_folder_sync (camel-folder.c:311)
> > ==15153==    by 0x414EB44: store_sync (camel-store.c:690)
> > 
> > 
> 
-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://www.pvanhoof.be/blog