Click here to Skip to main content
15,038,345 members
Please Sign up or sign in to vote.
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
Updated 13-Apr-13 22:51pm
v4
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.

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.
   
The caller of that method must delete (sooner or later) the memory. However that should be done somewhere inside the DLL itself.
   
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.
   
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;
}

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




CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900