CODEWITHSUNDEEP
memory-leaksc++-cli
Dororo
Dororo

Reputation: 3440

Identifying memory leaks in C++

I've got the following bit of code, which I've narrowed down to be causing a memory leak (that is, in Task Manager, the Private Working Set of memory increases with the same repeated input string). I understand the concepts of heaps and stacks for memory, as well as the general rules for avoiding memory leaks, but something somewhere is still going wrong:

while(!quit){
   char* thebuffer = new char[210]; 
   //checked the function, it isn't creating the leak
   int size = FuncToObtainInputTextFromApp(thebuffer); //stored in thebuffer
   string bufferstring = thebuffer;
   int startlog = bufferstring.find("$");
   int endlog = bufferstring.find("&");
   string str_text="";
   str_text = bufferstring.substr(startlog,endlog-startlog+1);
   String^ str_text_m = gcnew String(str_text_m.c_str());
   //some work done
   delete str_text_m;
   delete [] thebuffer; 
}

The only thing I can think of is it might be the creation of 'string str_text' since it never goes out of scope since it just reloops in the while? If so, how would I resolve that? Defining it outside the while loop wouldn't solve it since it'd also remain in scope then too. Any help would be greatly appreciated.

Upvotes: 1

Views: 1315

Answers (4)

ckv
ckv

Reputation: 10830

There is a tool called DevPartner which can catch all memory leaks at runtime. If you have the pdb for your application this will give you the line numbers in your application where all memory leak has been observed.

This is best used for really big applications.

Upvotes: 0

Ofek Shilon
Ofek Shilon

Reputation: 16129

Are you #using std, so that str_text's type is std::string? Maybe you meant to write -

String^ str_text_m = gcnew String(str_text.c_str());

(and not gcnew String(str_text_m.c_str()) ) ?

Most importantly, allocating a String (or any object) with gcnew is declaring that you will not be delete'ing it explicitly - you leave it up to the garbage collector. Not sure what happens if you do delete it (technically it's not even a pointer. Definitely does not reference anything on the CRT heap, where new/delete have power).

You can probably safely comment str_text_m's deletion. You can expect gradual memory increase (where the gcnew's accumulate) and sudden decreases (where the garbage collection kicks in) in some intervals.

Even better, you can probably reuse str_text_m, along the lines of -

    String^ str_text_m = gcnew String();
    while(!quit){
       ...
       str_text_m = String(str_text.c_str());
       ...
    }

Upvotes: 2

GManNickG
GManNickG

Reputation: 503845

You should use scope-bound resource management (also known as RAII), it's good practice in any case. Never allocate memory manually, keep it in an automatically allocated class that will clean up the resource for you in the destructor.

You code might read:

while(!quit)
{
    // completely safe, no leaks possible
    std::vector<char> thebuffer(210);
    int size = FuncToObtainInputTextFromApp(&thebuffer[0]);

    // you never used size, this should be better
    string bufferstring(thebuffer, size);

    // find does not return an int, but a size_t
    std::size_t startlog = bufferstring.find("$");
    std::size_t endlog = bufferstring.find("&");

    // why was this split across two lines?
    // there's also no checks to ensure the above find
    // calls worked, be careful
    string str_text = bufferstring.substr(startlog, endlog - startlog + 1);

    // why copy the string into a String? why not construct 
    // this directly?
    String^ str_text_m = gcnew String(str_text_m.c_str());

    // ...

    // don't really need to do that, I think,
    // it's garbage collected for a reason
    // delete str_text_m; 
}

The point is, you won't get memory leaks if you're ensured your resources are freed by themselves. Maybe the garbage collector is causing your leak detector to mis-fire.

On a side note, your code seems to have lots of unnecessary copying, you might want to rethink how many times you copy the string around. (For example, find "$" and "&" while it's in the vector, and just copy from there into str_text, no need for an intermediate copy.)

Upvotes: 4

HexBlit
HexBlit

Reputation: 1162

I know its recommended to set the freed variable to NULL after deleting it just to prevent any invalid memory reference. May help, may not.

delete [] thebuffer; 
thebuffer = NULL;     // Clear a to prevent using invalid memory reference

Upvotes: 1

Related Questions