HUGE MEMORY LEAK pinpointed!

[tiwag]

there is another memleak in codecompletion plugin (CCplugin)

loading and closing a big workspace (CB and all plugins projects e.g.),
with CCplugin enabled shows,
that the CCplugin leaks about 56MB of memory at each loading & closing cycle.

CB memory usage [MB]
with CCplugin
disabled   enabled      comment
37,7      34,7      initial startup of CB
39,9      150,5      load workspace
37,4      121,7      close ws
40,0      206,4      load workspace
37,8      183,2      close ws
40,1      262,1      load workspace
39,0      242,0      close ws
40,1      318,1      load workspace
38,9      299,4      close ws
40,3      373,8      load workspace
39,8      355,4      close ws
39,9      150,5      load workspace
40,5      429,7      close ws



tested with CB svn 4132, wx284 unicode, win-XP
memory usage measured with "Process Explorer" (sysinternals)
CCplugin set to parse local and global includes

brgds

[MortenMacFly]

there is another memleak in codecompletion plugin (CCplugin)

BT: I just remembered something I realised sometime back: A possible crash candidate in parser.cpp, lines 690-696 (and on some other places I don't recall, too):

Code Select Expand


  token = new Token();
  if (!token->SerializeIn(f))
  {
    delete token;
    token = 0;
    break;
  }

What if token could not be created (and is NULL therefore)? Shouldn't the if-check better be:

Code Select Expand

if (token && !token->SerializeIn(f))
or something? (Need to implement an appropriate action anyway then...)
With regards, Morten.
Ps.: Should be go for a corporate bug-hunting for CC only?! ;-)

[Der Meister]

there is another memleak in codecompletion plugin (CCplugin)

BT: I just remembered something I realised sometime back: A possible crash candidate in parser.cpp, lines 690-696 (and on some other places I don't recall, too):

Code Select Expand


  token = new Token();
  if (!token->SerializeIn(f))
  {
    delete token;
    token = 0;
    break;
  }

What if token could not be created (and is NULL therefore)?

token can't be NULL (or better: 0) even if it could not be created because new throws an exception (std::bad_alloc if I remember correctly) if it failes to allocate enough memory. Another possible problem could arise inside the construktor of the class Token, but again the only way for aborting it is throwing an exception. That means in the line "if (!token->SerializeIn(f))" token always points to a valid Token instance.

[mandrav]

there is another memleak in codecompletion plugin (CCplugin)

BT: I just remembered something I realised sometime back: A possible crash candidate in parser.cpp, lines 690-696 (and on some other places I don't recall, too):

Code Select Expand


  token = new Token();
  if (!token->SerializeIn(f))
  {
    delete token;
    token = 0;
    break;
  }

What if token could not be created (and is NULL therefore)? Shouldn't the if-check better be:

Code Select Expand

if (token && !token->SerializeIn(f))
or something? (Need to implement an appropriate action anyway then...)
With regards, Morten.
Ps.: Should be go for a corporate bug-hunting for CC only?! ;-)

The serialization functions are not used anymore (for a long time now).
This means ReadFromCache() and WriteToCache() are not used so what you pointed at is effectively dead code.

[Biplab]

Here is some "Meat" for "Memory-Leak-Hunters".

I'm attaching the Valgrind log of Code::Blocks. I've run debug build of C::B (Rev 4145) with valgrind with the following options.

Code Select Expand

valgrind --leak-check=full --show-reachable=yes

BTW, I'm a newbie with valgrind. So I'm posting the log which I received. Please feel free to suggest improvements, if any.

Best Regards,

Biplab

[attachment deleted by admin]

[rickg22]

w00t! Excellent!

Now... how do we interpret it?

[rickg22]

Oh my, look at this.

==29618== 10,748,090 bytes in 774 blocks are indirectly lost in loss record 244 of 244
==29618==    at 0x400591C: operator new[](unsigned) (vg_replace_malloc.c:195)
==29618==    by 0x49D3584: FileLoader::operator()() (filemanager.cpp:47)
==29618==    by 0x49D5727: BackgroundThread::Entry() (backgroundthread.h:153)
==29618==    by 0x4286217: wxThreadInternal::PthreadStart(wxThread*) (in /usr/local/lib/libwx_gtk2u-2.8.so.0.1.1)
==29618==    by 0x428628C: wxPthreadStart (in /usr/local/lib/libwx_gtk2u-2.8.so.0.1.1)
==29618==    by 0x48E532FA: start_thread (in /lib/libpthread-2.6.so)
==29618==    by 0x42CC993D: clone (in /lib/libc-2.6.so)

There's the culprit! According to the report, there is a new[] without delete[] in FileLoader::operator()().

Ah, there it is. Since it's multithreaded, it does NOT dispose the memory when the file's loaded successfully. But then it's the client which must do it, right? So that means code completion does NOT dispose the data once it's read... or does it?

[Biplab]

There's the culprit! According to the report, there is a new[] without delete[] in FileLoader::operator()().

Great Ricks.

I hope this report can be used to clean up memory leak to a great extent.

BTW, I forgot to mention that to generate this report, I opened CodeBlocks-unix.cbp file with about 5-6 files open in Editor. Then I allowed Code Completion plugin to parse the project. Then I closed the project and C::B.

[rickg22]

I have a question.

There's a Delete( ) operation done.

But I can't find ANYWHERE the function "Delete" (with uppercase). Is that C++? because I'd only seen delete in lowercase. Now I'm confused 

[Biplab]

There's a Delete( ) operation done.

I believe this is for thread deletion, not for deallocating memory.

[rickg22]

I'm really not sure, ParserThread is cbThreadedTask and not wxThread. So Delete() can't really belong to wxThread.

[Biplab]

I'm really not sure, ParserThread is cbThreadedTask and not wxThread. So Delete() can't really belong to wxThread.

Where is this exactly?

[TDragon]

You guys are barking up the wrong tree if you're talking about that new[] leak.

I think the problem is that the destructor in LoaderBase, which is responsible for delete[]-ing data, isn't virtual. In sdk/filemanager.cpp:139,143, FileLoader* are pushed onto BackgroundThread queues. Then in include/backgroundthread.h:145-158 they're popped off as AbstractJob* and, if owned by the BackgroundThread, deleted. The LoaderBase destructor isn't in the object's vtable and we're delete-ing by way of an AbstractJob*, so ~LoaderBase() never happens and we have a missing delete[].

Edit:
Nope, I was wrong; a base class' virtual destructor forces derived classes' destructors to be virtual also.

[Der Meister]

As there is a delete[] for this reported memory leak located in the destructor of LoaderBase I think there are only two possible problems:
- The destructor does not get called.
- FileLoader::operator() gets called more than once before the instance is destroyed. This way the operator() would just request more memory but not release the old one.

I don't know which of this two things happens but I hope for the second one as it should be easy to solve. I'm just running valgrind to check this.

[rickg22]

Thomas, any ideas?