Clean Up Memory Allocation Problems

Question

1.00/5 (1 vote)

See more:

Hi There,
I have "adopted" some code that does some fancy stuff with graphics.
It is written in C++ and compiled into a callable DLL that I can use in my C# program.
(all the c++ source is available)

It works great and gives me exactly the features I need but it leaks memory like a sieve.
I figure I can fix it and give something back to the open source community as the original author is too busy with real work at the moment. :)

The trouble is my C++ is fairly average but hey I can google. I have fixed one gusher but I see a few blocks of code that look like this for example.

C++

ImageData* ImageData::Clone()
{
	ImageData* img_data = new ImageData;		
	img_data->m_bytesPerPixel = m_bytesPerPixel;
	img_data->AllocDataSpace(GetWidth(), GetHeight(), GetBytesPerPixel() * 8);
	memcpy(img_data->m_data, m_data, GetSize());		
	img_data->m_width = m_width;
	img_data->m_height = m_height;

	return img_data;
}

My understanding is that the "new" in the first line is going to allocate some memory and there has to be a corresponding "delete img_data" somewhere else to deallocate the memory.
I searched the project and there isn't one.

I'd expect to be doing the delete in the same method that created it.
But img_data is being returned to the calling method so I can't delete it there.

I'm assuming that this is a leak but I don't know how to deal with it.
Any sugestions / points in the right direction would be very helpfull.

Thanks in Advance
David

Update 14Apr13
Perhaps I didn't make this clear enough. Here are some more details. My c# code passes a file name to the c++ code contained within the DLL. The code in the DLL calculates if the document in the file has been scanned straight and returnes the angle of the document or 0 if it is square to the c# code. So clearly the problems lies within the c++ code.
Further investigation revealed that the example module shown below is never actually called.
If anyone would like to look at the code it is in this Code Project Article and a simple example VIsual Studio project to call and test the DLL is included.

Detect image skew angle and deskew image[^]

Thanks for all your comments so far

Posted 9-Apr-13 21:28pm

David_Pollard

Updated 13-Apr-13 22:51pm

v4

Add a Solution

Comments

Philippe Mori 13-Apr-13 18:58pm

Given that you are new to C++ and that you don't understand well who is responsible to delete the memory, I would recommand you not to touch the Library for a while. You should first learn memory managment. Learning how to use smart pointers might also be helpful to simplify memory managment. It is important that you identify the memory owner or that you uses shared pointers.

3 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

H.Brydon · Answer 1 · 2013-04-12T08:20:00

When you call an API to make a clone of something else then really, the calling code becomes the owner of the clone. It is the responsibility of the calling code to clean up after itself. If a memory leak occurs, the problem lies with the caller, not with the clone factory. There is no amount of "fixing" that will make this better.

I would advise against doing something automatic with the memory when the 'factory' DLL exits or any other time. The job of the factory method is to produce the clone, not to manage its lifetime. If any objects are deleted here, other bad things can happen.

Solution #1 is correct but perhaps not completely stated. I would suggest that Solution #2 is a bunch of nonsense.

If you want to find and fix any apparent memory leaks, figure out where the calling code is done with the clone and delete it there; nowhere else.

CPallini · Answer 2 · 2013-04-09T21:37:00

Solution 1

The caller of that method must delete (sooner or later) the memory. However that should be done somewhere inside the DLL itself.

Posted 9-Apr-13 21:37pm

CPallini

Lakamraju Raghuram · Answer 3 · 2013-04-09T22:15:00

Solution 2

To elaborate CPallini:

In the same DLL, you can add another function say FreeMem(void* p) (which will call delete on p) and once the caller of ImageData::Clone() is done with his work, he can call FreeMem(imageData) to free the memory.

Posted 9-Apr-13 22:15pm

Lakamraju Raghuram

Comments

David_Pollard 10-Apr-13 4:58am

Ok Thanks that makes some kind of sense. I can fix the code within the Dll.
Should the calling method also allocate the memory or is that ok left where it is?
How should the delete look? I don't want to just delete a pointer to the memory.
Sorry big newbie question I know.

Stefan_Lang 10-Apr-13 6:06am

Freeing the memory should always happen in the same library (or application) that allocates it. See my the other comment to this solution (I accidentally posted it as comment rather than as a reply to your comment...)

Stefan_Lang 10-Apr-13 6:03am

As suggested here, the DLL that performs the new, also must call the delete. If there is no function already, it needs to be added there. The call to FreeMem() (or whatever the function is called) should then be performed in the same application or library that calls the Clone() method which does the allocation.

An implementation inside the class ImageData could look like this:
class ImageData {
...
static void FreeMem(ImageData*& p) {
delete p;
p = 0;
}
...
};

To use it, you'd call it like this:

void foo(ImageData* orig) {
ImageData* myimg = orig->Clone();
... // do something
ImageData::FreeMem(myimg);
}

David_Pollard 10-Apr-13 6:21am

This will take me some trial and error. & and * together confuse the crap out of me.
I have created a bare bones (c++) console app for testing. I should be able to create the same leak in that and then fix it. If I come with a easily understandable example I'll post here.

David_Pollard 10-Apr-13 20:58pm

OK I have come up with a very simple example program.
The Main method calls Sub1 method to create an array.
The Main method outputs something from the array.
The Main method calls freeMem method to delete the array.
I don't know what I'm doing trying to pass pointers back and forward.
I have included the compile errors as comments below.
If I can get this example to work I should be able to extrapolate it to my actual problem.
Thanks
David

// TestMemoryLeak.cpp : main project file.

#include "stdafx.h"

// Added the follwoing lines for Memory Leak Detection
// Turned off Precompiled Headers in project options
#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>

// Refifines "new" when in debug mode
#ifdef _DEBUG
#ifndef DBG_NEW
#define DBG_NEW new ( _NORMAL_BLOCK , __FILE__ , __LINE__ )
#define new DBG_NEW
#endif
#endif
// End Memory Leak Detection Additions

using namespace System;
#define byte unsigned char

byte* sub1(int arraySize);
void freeMem(int pArray[]); // <-- Just guessing here

int main(array<system::string ^=""> ^args)
{
// Sets memory debug the output to go to the Output Window
_CrtSetReportMode( _CRT_WARN, _CRTDBG_MODE_DEBUG );
int arraySize= 1000;
byte pByteArray;

// Create and Save stuff in the array
pByteArray = sub1(arraySize); //Error 1 error C2440: '=' : cannot convert from 'unsigned char *' to 'unsigned char'

//Just display something from the array
Console::WriteLine(&pByteArray[20]); //Error 2 error C2109: subscript requires array or pointer type

//Free the Memory
freeMem(pByteArray); //Error 3 error C2664: 'freeMem' : cannot convert parameter 1 from 'unsigned char' to 'int []'

// Output the memory leak debug info now.
_CrtDumpMemoryLeaks();
return 0;
}

byte* sub1(int arraySize)
{
// Create the Array
byte* byteArray = new byte [arraySize];
for(int count=1; count < arraySize; count++)
{
// Put stuff in array
byteArray[count]=120; //Just save something in the array.
}
return byteArray;
}
void freeMem(int pArray[])
{
// Destory the array
// or comment to make a leak
delete pArray; //Error 3 error C2541: 'delete' : cannot delete objects that are not pointers

pArray=0;
}