Re: [Tracker] Memory leak in tracker-indexer

[Martyn Russell <martyn imendio com>]

Laurent Aguerreche wrote:
> Hello,
>
> Le jeudi 25 septembre 2008 Ã 13:48 +0200, laurent aguerreche free fr a
> Ãcrit :
> > Selon Carlos Garnacho <carlos imendio com>:
> >
> > > On miÃÂ, 2008-09-24 at 23:56 +0200, Laurent Aguerreche wrote:
> > > > Le mercredi 17 septembre 2008 Ã  11:02 +0100, Martyn Russell a ÃÂcrit :
> > > > > Laurent Aguerreche wrote:
> > > > > > Hello,
> > > > Hello,
> > Hello,
> >
> > > > > > there is a big memory leak with tracker-indexer. The processus uses
> > > more
> > > > > > and more memory but free almost all memory when it exists so I assume
> > > > > > that a list of extracted info is not freed while tracker-indexer is
> > > > > > running.
> > > > > Hi,
> > > > >
> > > > > I thought I would check up on this. My indexer was running at ~200Mb. I
> > > > > found 3 or 4 leaks so far in code which is run thousands of times. I
> > > > > will commit the fixes when I finish valgrinding.
> > > > I still see a big memory leak when tracker-indexer starts to treat
> > > > Evolution emails. Tracker-indexer uses more than 1GB after less than one
> > > > hour of Evolution emails indexing and so I have to kill trackerd.
> > > IMAP or mbox? the code paths are quite different
> > IMAP !
>
> I was fed up with slowdowns from tracker so I decided to run gdb when
> tracker-indexer hit 1600Mo (yes, really 1,6Go) of resident memory and I
> obtained that :
>
>
> (gdb) bt
> #0  0x00000038fbc82f1d in memset () from /lib64/libc.so.6
> #1  0x000000304523f6db in g_try_malloc0 () from /lib64/libglib-2.0.so.0
> #2  0x00000000044f8d1f in read_summary (summary=0x7fab9b755360) at
> evolution.c:209
> #3  0x00000000044fb381 in tracker_module_file_iter_contents
> (file=0x2239f60) at evolution.c:1593
> #4  0x000000000040e7ee in tracker_indexer_module_file_iter_contents
> (module=0x1d8bc60, file=0x2239f60) at tracker-indexer-module.c:212
> #5  0x000000000040b54e in process_file (indexer=0x1d84800,
> info=0x20ce340) at tracker-indexer.c:1927
> #6  0x000000000040b9df in process_func (data=0x1d84800) at
> tracker-indexer.c:2101
> #7  0x000000304523742b in g_main_context_dispatch ()
> from /lib64/libglib-2.0.so.0
> #8  0x000000304523ac0d in ?? () from /lib64/libglib-2.0.so.0
> #9  0x000000304523b13d in g_main_loop_run ()
> from /lib64/libglib-2.0.so.0
> #10 0x000000000040fa2d in main (argc=1, argv=0x7fffa8f94ae8) at
> tracker-main.c:364
> (gdb) frame 2
> #2  0x00000000044f8d1f in read_summary (summary=0x7fab9b755360) at
> evolution.c:209
> 209                           str = g_try_malloc0 (len);
> (gdb) print str
> $1 = (gchar *) 0x55 <Address 0x55 out of bounds>
> (gdb) list
> 204   
> 205                           if (len <= 1) {
> 206                                   continue;
> 207                           }
> 208   
> 209                           str = g_try_malloc0 (len);
> 210                           if (!str) {
> 211                                   return FALSE;
> 212                           }
> 213   
> (gdb) 
> 214                           if (fread (str, len - 1, 1, summary) != 1) {
> 215                                   g_free (str);
> 216                                   return FALSE;
> 217                           }
> 218   
> 219                           if (dest) {
> 220                                   *dest = str;
> 221                           } else {
> 222                                   g_free (str);
> 223                           }
> (gdb) print len
> $2 = 2834908688
> (gdb) 
>
>
>
> So summary file is still not correctly parsed. Since I use a 64 bits
> system and that you do not seem to have this problem, I suspect a wrong
> assumption with size of types like it happened with time_t. Do you
> remember any assumption you made with size of types?

We have had this issue before. That is why we now use g_try_malloc0() so
we don't just crash when we can't allocate the memory. I know this isn't
the best solution but I too have seen it eat 1.5gb of memory (not for me
anymore, but then I don't run 64bit) but why don't we add a simple check
in here and if len is > some arbitrary figure then we assume it is crack
and bomb out there and then? This figure could be 100Mb or something?

Of course the real solution is to fix it but I fear this problem may
persist because from what I hear, passing these files is a dark art and
if you get one part of it wrong then this sort of issue can come up
again very easily. We should probably add some checks to prevent this
happening.

-- 
Regards,
Martyn