🔒 Locked

[VC++] Memory Leaks

Started by Ripiz Oct 16, 2009 at 6:39 AM 32 replies 5k views

Original Post

Oct 16, 2009 at 6:39 AM

I found this FAQ about Memory Leaks and tried to use it in my own program


//Main.h
#include <windows.h>
#include <d3d9.h>
#include <d3dx9.h>
#include <dinput.h>
#include <iostream> 
#include <fstream>
#include <time.h>
#include <vector>
#define _CRTDBG_MAP_ALLOC
#include <crtdbg.h>
/* other includes and variable definitions */

According to that FAQ it should display file where leak happened, however it remains undetailed:


{1445} normal block at 0x01ECC338, 4160 bytes long.
 Data: <@               > 40 10 00 00 08 00 00 00 CD CD CD CD CD CD CD CD 
{1444} normal block at 0x01ECB2B8, 4160 bytes long.
 Data: <@               > 40 10 00 00 08 00 00 00 CD CD CD CD CD CD CD CD 
{1443} normal block at 0x01E8B238, 262208 bytes long.
 Data: <@               > 40 00 04 00 08 00 00 00 CD CD CD CD CD CD CD CD 
{1442} normal block at 0x01E871F8, 16384 bytes long.
 Data: < @       r      > 00 40 00 00 08 00 00 00 E0 72 E8 01 00 00 00 00 
{1441} normal block at 0x01E85478, 7488 bytes long.
 Data: <@           `F  > 40 1D 00 00 08 00 00 00 02 00 00 00 60 46 FF 05

Could anyone help me please? Thank you in advance

_moagstar_

Oct 16, 2009 at 6:50 AM

You need to overload the new operator to inject the filename and line number into the allocation :

#include <crtdbg.h>#pragma warning(disable:4291)void* operator new(size_t size, const char * file, const int line){	return ::operator new(size, _NORMAL_BLOCK, file, line);}void* operator new[](size_t size, const char * file, const int line){	return ::operator new[](size, _NORMAL_BLOCK, file, line);}#ifdef _DEBUG#	define DBG_NEW new(__FILE__, __LINE__)#else#	define DBG_NEW new#endif

You can then use DBG_NEW in place of new However this won't catch any leaks caused in 3rd party code.

Ripiz

Oct 16, 2009 at 7:11 AM

//I have this in every file#define CRTDBG_MAP_ALLOC#include <stdlib.h>#include <crtdbg.h>//And this in one of filesvoid* operator new(size_t size, const char * file, const int line){	return ::operator new(size, _NORMAL_BLOCK, file, line);}void* operator new[](size_t size, const char * file, const int line){	return ::operator new[](size, _NORMAL_BLOCK, file, line);}#define DBG_NEW new(__FILE__, __LINE__)

However it still output it without ability to detect where leak happens:

{1445} normal block at 0x01E7B528, 4160 bytes long. Data: <                > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD {1444} normal block at 0x01E3B4A8, 262208 bytes long. Data: <                > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD {1443} normal block at 0x01E37468, 16384 bytes long. Data: <                > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD {1442} normal block at 0x01E356E8, 7488 bytes long. Data: <                > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD {153} normal block at 0x00C54E38, 2864 bytes long. Data: <                > B4 FE DC 00 01 00 00 01 00 00 00 00 00 00 00 00

Possibly I did something wrong, I'm not very good at C++ yet. Thank you in advance.

_moagstar_

Oct 16, 2009 at 7:12 AM

This will only work if you actually replace the allocations that you want to check with DBG_NEW instead of new

Ripiz

Oct 16, 2009 at 7:21 AM

Sounds complicated... But thanks, I will try.

_moagstar_

Oct 16, 2009 at 7:32 AM

Quote:
Original post by Ripiz
Sounds complicated... But thanks, I will try.

It's fairly straightforward actually, there shouldn't really be that many places where you use new, try a find in files to find all the places and replace those.

Alternatively you could do the following at the top of each of your cpp files (after including any other headers) :

#define new new(__FILE__, __LINE__)

And let the preprocessor do the replacing for you. I wouldn't recommend leaving this code in though, it's a little bit dirty but should help you track the source of your leaks.

Ripiz

Oct 16, 2009 at 7:37 AM

Somewhere I found _CrtSetBreakAlloc(long);. It seems to be able to show where leak happens, but it opens include file, and I have to Step Out many times, and sometimes it doesn't show me the place.
Is it good way to find leaks?

_moagstar_

Oct 16, 2009 at 7:44 AM

Yes this is another strategy you can use if you call _CrtSetBreakAlloc(1445) (from your example output) then the IDE should break into the debugger when that allocation happens.

You shouldn't have to step out, take a look at the call stack and find the last function that is one of yours, hopefully that should be one of the allocations that is causing the leak.

Rinse and repeat. I personally think that using debug new to inject the file and line information into the allocation is easier, but whatever works best for you really :) good luck.

Ripiz

Oct 16, 2009 at 7:50 AM

I don't know how to check last function call, but as far as I understand, Stepping Out will continue code execution, and eventually will lead me back to where it was called, to continue execution of lines after that call.

Also I encountered problem:

Model::Model(char *File, Vector vp, Vector vr, Vector vs){	m_filename="data/";	m_filename+=File;	vecRot=vr;	vecPos=vp;	vecScale=vs;	loaded=false;	alphaLevel=NULL;	alpha=NULL;	Mesh=NULL;	idle=NULL;/*one of problems happened with File, I tried free(File), delete File, delete [] File but I keep getting program crashed. Any tips?*/}

_moagstar_

Oct 16, 2009 at 8:17 AM

Debug Menu->Windows->Call Stack.

This should give you a list of the functions that were called in order to get to the current point of execution. In that list should probably be lots of functions that aren't yours, and then further down you should start seeing your functions. Double click on the first function in the list that is yours, hopefully this should show you the allocation which is causing the leak.

/*one of problems happened with File, I tried free(File), delete File, delete [] File but I keep getting program crashed. Any tips?*/

Use std::string...

Out of curiosity...what is the type of m_filename?

Buckeye

Oct 16, 2009 at 8:26 AM

Quote:
I don't know how to check last function call

If you're using Visual Studio, either:

1. click on the Call Stack tab at the bottom of the IDE (later versions of VS)

or

2. in the menu, click Debug->Windows->Call Stack.

A window listing functions being executed should appear. As moagstar suggested, scan the list from the top down and find the first function that's in your code. Double-click that line. Your code file will pop up with an arrow pointing to the line where the error occurred.

With regard to deleting a char*:

1. The function that calls the Model constructor should be responsible for disposing of the pointer.

2. How is the char* allocated? If you're just passing the address of a fixed buffer, don't delete it! You only delete things that you new'd.

For instance:

char File[256];
// get the filename
Model *mdl = new Model(File, ... );

Don't delete File! It will be deallocated when it goes out of scope.

Ripiz

Oct 16, 2009 at 8:33 AM

For some reason Call Stack only starts showing from the place where debugger stopped execution (http://img225.imageshack.us/img225/4930/44054795.jpg)

class Model{	public:		Model(char *File,Vector vp, Vector vr, Vector vs);		~Model();		void Draw();		void Update(float Time);		void Load();		Vector vecRot;		Vector vecPos;		Vector vecScale;		float alphaLevel;		bool alpha;		ID3DXMesh *Mesh;		bool loaded;	private:		vector<Material> Mtrls;		string m_filename;		float idle;};//calling constructortest=new Model("bones_all.x",Vector(0,0,0),Vector(0,0,0),Vector(1,1,1));

Another problem:

if(mat->texture==NULL){	Texture *tex=new Texture(); //leak here, so I assume Textures vector isn't getting cleaned up	tex->filename="data/";	tex->filename+=mtrls.pTextureFilename;	tex->loaded=false;	mat->texture=tex;	Textures.push_back(tex);}//vector definitionvector<Texture*> Textures;//Texture structstruct Texture{	string filename;	bool loaded;	float time;	IDirect3DTexture9 *texture;	~Texture(){		if(loaded)texture->Release();	}};

My code is very dirty, I'm still learning and kinda never bothered with cleaning up memory =/ Guess I have to do it now.

Thank you in advance

david_watt78

Oct 16, 2009 at 8:46 AM

Memory leaks are for the most part easy to avoid if you use a smart pointer class and object reference counting. there are 2 ways to implement it. The first way is to use a static map in the smart pointer template. The second is the derive all your classes from a common base with an Increment and Decrement the count functions. Either way, every smart pointer that receives an address will increment its count when its assigned and decrement the count if it receives a new address or is destroyed. When an address's count < 0 you can delete the address. As a note the map method is more expensive to use but will work for any object type. Also avoid the 2 objects holding pointers to each other scenario as neither will be released in that case.

Alex

Oct 16, 2009 at 8:47 AM

I haven't read the whole post, so I don't know if you got it working or not, but I figured I'd show you this tool:

Visual Leak Detector

I have found it extremely useful for finding memory leaks in my projects, and it is extremely simple to use, just include the vld.h header in your project and it does the rest.

Buckeye

Oct 16, 2009 at 8:48 AM

Reread what moagstar and I suggested. In the image you posted, if you start at the top of the list and scan down, isn't Client.exe!Model::Load() your code? Double-click on that line.

Quote:
test=new Model("bones_all.x",Vector(0,0,0),Vector(0,0,0),Vector(1,1,1));

"bones_all.x" is actually stored in your code module. You didn't new it; don't delete it.

In your code for if(mat->texture==NULL), you have:

mat->texture=tex;Textures.push_back(tex);

You've stored the new Texture in two places. Make sure you only delete it once.

Suggestion for your Texture structure: ALWAYS initialize pointers.

struct Texture {
Texture() { texture = NULL; }
~Texture() { if(texture != NULL) texture->Release(); }
//.. etc.
}

_moagstar_

Oct 16, 2009 at 9:02 AM

Quote:
Original post by david_watt78
Memory leaks are for the most part easy to avoid if you use a smart pointer class and object reference counting.

This is a great suggestion and would probably solve all of the OP's problems with one foul swoop, however if all you are interested in is resolving your memory leaks then I would suggest looking at tr1::shared_ptr or boost::shared_ptr rather than rolling your own...If you want to learn about how to implement a smart pointer, then by all means roll your own.

However before doing either of these I would recommend that you try and fix these memory leaks, even if it's just to prove to yourself how much of a pain in the ass worrying about memory allocation / deallocation is, and also to get yourself into the mindset that if you allocate it, you need to deallocate it, and implementing the allocation at the same time as the deallocation is the easiest way of achieving this, procrastination just makes this kind of thing harder.

Ripiz

Oct 16, 2009 at 9:14 AM

I will try to make deallocation when I make allocation :) This stuff will teach me how not to be lazy.

{1520} normal block at 0x05FB31E8, 32 bytes long.
Data: 64 61 74 61 2F 62 6F 6E 65 73 5F 61 6C 6C 2E 78

string m_filename; in class Model doesn't seem to clean up. =/ Weird...

Visual Leak Detector doesn't detect any leaks =/

Edit:

for(DWORD i=0;i<NumMtrls;i++){		Material *mat=new Material();	/*skipped*/	Mtrls.push_back(*mat);}struct Texture{	string filename;	float time;	IDirect3DTexture9 *texture;	Texture(){		texture=NULL;	}	~Texture(){		if(texture!=NULL)texture->Release();	}};struct Material:D3DMATERIAL9{	Texture *texture;	Material(){		texture=NULL;	}	~Material(){	}};

How to build deconstructor for Material? texture->Release() doesn't work and it makes many leaks (over 90% of all leaks) with below 100 bytes each

[Edited by - Ripiz on October 16, 2009 9:14:52 AM]

Buckeye

Oct 16, 2009 at 9:47 AM

How do you delete the Textures you're storing in your Textures vector?

Maybe:

for(int i=0; i<(int)Mtrls.size(); i++) {   if( Mtrls->pTextureFilename != NULL ) delete Mtrls->pTextureFilename; // assumes that pTextureFilename = new char[...] somewhere   delete Mtrls;}// let the Texture class release the texture in its destructorfor(int i=0; i<(int)Textures.size(); i++) delete Textures;

Ripiz

Oct 16, 2009 at 9:57 AM

I'm not sure what you mean. I'm quite dumb =/

I need to remove every 'Material' in 'vector Mtrls;' but not to delete 'Material.texture', as it points to 'Texture' which is also used by other 'Material's.

Buckeye

Oct 16, 2009 at 10:01 AM

Quote:
I need to remove every 'Material' in 'vector Mtrls;' but not to delete 'Material.texture', as it points to 'Texture' which is also used by other 'Material's.

Correct. Deleting a Material does not delete the Texture. It just releases the memory allocated for the pointer to the Texture. Until you delete the elements of the Textures vector, the pointers in the other Material's will still be valid.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.