Skip to main content
GameDev.net gamedev.net
🔒 Locked

Memory Leaks - Was Confused But Now Seems clearer

Started by Xrystal Nov 12, 2006 at 4:58 PM 14 replies 3.7k views
Original Post
Xrystal
Xrystal
I've rigged up a program and using the DX debugger I have as far as I can see removed all the memory leaks. The Debug output shows memfini with no mention of memory leaks. Until that is when I added an effects class to handle effect files and their use in my primitive class. It is now throwing out the following message: Direct3D9: (INFO) :MemFini! Direct3D9: (WARN) :Memory still allocated! Alloc count = 508 The number changes the longer the program runs of course but I can't seem to find out where the leak is. I have a release function that releases, deletes and/or nulls any variables it uses but it still has something there. Now, after reading the forums I stumbled across a post that pointed out some CRT functions so started with : _CrtSetDbgFlag(_CrtSetDbgFlag(_CRTDBG_REPORT_FLAG) | _CRTDBG_LEAK_CHECK_DF); in my main function and it trapped almost 1000 of these apparent memory leaks. I then used : _CrtSetBreakAlloc(44); To break on the first apparent memory leak. Which happened to be my first line of code *sigh*. CDXEngine* lEngine = new CDXEngine(); Why would it think that was a memory leak. Thats simply the engine class being initialised. I tried a few more and so far only lines of code using 'new' are being tracked. Where are the memory leaks ? How can I find out memory leaks inside a single class where I think the problems are ? Why the heck couldn't they give you line numbers or source files to give you a head start :D Thanks for reading this garbled message. And thanks in advance for any ideas. edit: This seems to be the block of code at fault.

	if (m_bHasEffects)
	{
		//if (!m_pEffect->CreateFromFile()) return false;
		//m_pEffect->UseTechnique();
		//int passes = m_pEffect->Begin();
		//for (int i = 0; i < passes; i++)
		//{
		//	m_pEffect->SetConstants();
		//	m_pEffect->BeginPass(i);
			if (m_bHasIndices) hr = m_pD3DDevice->DrawIndexedPrimitive(type,0,0,m_iCount,0,count);
			if (!m_bHasIndices) hr = m_pD3DDevice->DrawPrimitive(type,0,count);
			if (hr == D3DERR_INVALIDCALL)
			{
				MessageBox(NULL,"Invalid Call","Drawing Primitive",MB_OK);
				return false;
			}
		//	m_pEffect->EndPass();
		//}
		//m_pEffect->End();
	}


In that format no memory leaks appear (using just the dx debugger). But simply by un remming the CreateFromFile line I get the memory leaks. This is all that routine does.

	HRESULT hr = D3DXCreateEffectFromFile(m_pD3DDevice,m_szFileName,
							 m_pD3DDefines,m_pD3DInclude,
							 m_dwShaderFlags,m_pD3DPool,
							 &m_pD3DEffect,&m_pD3DCompileErrors);
	if (hr == D3D_OK) 
	{
		m_bCreated = true;
		return true;
	}
	if (hr == D3DERR_INVALIDCALL)
	{
		MessageBox(NULL,"Invalid Call","Create Effect from File",MB_OK);
		return false;
	}
	if (hr == D3DXERR_INVALIDDATA)
	{
		MessageBox(NULL,"Invalid Data","Create Effect from File",MB_OK);
		return false;
	}
	if (hr == E_OUTOFMEMORY) 
	{
		MessageBox(NULL,"Out of Memory","Create Effect from File",MB_OK);
		return false;
	}
	if (!m_pD3DEffect) return false;


I can't see anything wrong with it. All the class m_ variables are initialised to 0 or in the case of the device and filename the values passed when the class was initialised. Unless it is a problem with DX itself *laughs*. [Edited by - Xrystal on November 14, 2006 7:36:57 PM]
Xrystal
Xrystal
I'm running DirectX in debug mode and when the program finished it outputs some debug information:

Direct3D9: (INFO) :MemFini!
Direct3D9: (WARN) :Memory still allocated! Alloc count = 912
Direct3D9: (WARN) :Current Process (pid) = 00000938
Unhandled exception at 0x7c901230 (ntdll.dll) in U06_DirectX_Shaders.exe: User breakpoint.
The program '[2360] U06_DirectX_Shaders.exe: Native' has exited with code 0 (0x0).

On reading other posts that have been posted in the last 2 years it seems the CreateEffectFromFile line seems to be having a problem. And looks like they somehow fixed it in Managed DirectX as I didn't have this problem in that.

I am currently using Visual C++ 2003 Unmanaged DirectX October 2006 Version.

It is starting to look like I need to investigate why we are finding errors on this function. Kinda glad I'm not the only one having problems. Gonna go hunt more posts and see if someone mentions the cause.

Thanks for the quick reply.
Zahlman
Zahlman
Just wanted to say that I love how it reports "MemFini!" It's as if they ran out of memory before they could fit in the 'sh' :)
streamer
streamer
It is very simple every object that reserves memory with new must be deleted, or memory stays unfreed.
CDXEngine* lEngine = new CDXEngine();

must have

delete lEngine;

before program exits. And if you reserve lot of memory with new and you don't delete them, you will have lot of memory leaks!
Xrystal
Xrystal
Yep thats what I've been painstakingly doing with my code. Until I added the latest class - the effect class - there were no memory leaks showing on the debug screen.

However, the problem only occurs when running with the DX Control Panel showing as debug mode so perhaps this is a Debug DirectX problem only.

I'll take another look at all my 'new' variables and check if I missed some during my checks.

Thanks for the reply.


edit:

*Shuffles feet* erm .. er .. seems I didn't use delete for non array variables. I was NULLing them and Releasing their class variables but not actually deleting them.

It doesn't seem to have changed anything but that could be because I am somehow not calling the relevant code. Will have to check each class now. Really glad that I kept them small :D

Thanks again.

edit 2 :

Well, had to restore back to the code I had before adding the effects changes. Should have made a copy before doing the 'delete additions' as the program now asserts and crashes with an error inside the dbgdel.cpp file which has nothing to do with me.

_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)
dbgdel.cpp (52)

This contains the following block of code:
void operator delete(        void *pUserData        ){        _CrtMemBlockHeader * pHead;        RTCCALLBACK(_RTC_Free_hook, (pUserData, 0));        if (pUserData == NULL)            return;        _mlock(_HEAP_LOCK);  /* block other threads */        __TRY            /* get a pointer to memory block header */            pHead = pHdr(pUserData);             /* verify block type */            _ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));            _free_dbg( pUserData, pHead->nBlockUse );        __FINALLY            _munlock(_HEAP_LOCK);  /* release other threads */        __END_TRY_FINALLY        return;}


Obviously one of the deletes I am using is wrong but they are being called at the destructor point of each class to ensure that if the object exists it is deleted.

With the backup code and using the _crt debug code (which finally includes line nos) the debug report is showing the errors occuring inside the crtdebug file itself. Again outside of my own code.

I am now starting to wonder if directx itself is at fault.

Oh well, guess its back to square one and figuring out when and where the delete options should appear without crashing.

[Edited by - Xrystal on November 14, 2006 3:36:32 PM]
NotAYakk
NotAYakk
Start using reference counting pointers. Never new a pointer without storing it in a reference counted pointer. Avoid storing non-reference-counted pointers for any period of time, unless you can prove relative lifetimes.

This won't solve all of your memory leak problems, but it will catch 95% of them.

The remaining ones are known as "circular references". They are fun to deal with.

;)
Xrystal
Xrystal
hmm never come across 'reference counted pointers' before. Are they those std things as I've never dabbled with the std namespace.

Most of my 'new' statements are creating an instance of my own classes which I then (now at any rate) delete when its owner class is finished with it.

The only thing I can think of doing to change my program to not use:

CDXEngine* lpEngine = new CDXEngine();
delete lpEngine;

Is to use :

CDXEngine lpEngine = CDXEngine();

Which I assume won't need to be deleted but all those -> throughout the project for these 'new' objects will need to be replaced with '.'. Not a pleasant task.

As far as I can see I am now 'deleting' all 'new' objects so cannot understand why it is insisting on finding fault when in debug mode.

Well, I've wasted tonights free time trying to figure this out and have not moved a step forward after 5hrs of debugging to no avail. Kinda frustrating.

And all this only surfacing because I was debugging a DX Shader test run which it now seems is not related to the shader at all and didn't exist until after I added the 'delete' lines in.


Thanks for your reply. Will have to see what these pointers are. I've only ever known of pointers as a single entity and not multiple types.
Xrystal
Xrystal
Aha .. finally have it sorted .. at least in the project before the effects class was added.

The last problem I found was caused by deleting an object that wasn't a new object. Had to use the Release option as they were DX interface objects.

So, looks like I have all the relevant new/delete objects set up. Just have to remember to keep doing that as required as I add new classes.

Thanks again for all your assistance.
NotAYakk
NotAYakk
Basically, a reference counted pointer is a class that acts like a pointer to a different object, and holds a reference count.

One example is boost::shared_ptr.
Julian90
Julian90
Quote:
Are they those std things as I've never dabbled with the std namespace.


The SC++L is your friend, use it, it will save you alot of time.
streamer
streamer
Also instead of CDXEngine lpEngine = CDXEngine();
you can write CDXEngine lpEngine;
because default constructor is automaticly called;
NotAYakk
NotAYakk
Quote:
Original post by streamer
Also instead of CDXEngine lpEngine = CDXEngine();
you can write CDXEngine lpEngine;
because default constructor is automaticly called;


CDXEngine lpEngine = CDXEngine();
will actually create a CDXEngine, then do a copy-construction of a CDXEngine, then call the destructor of your first CDXEngine.

Your CDXEngine -- does it have a non-trivial constructor?
Does it have a non-trivial destructor?
Did you write a copy constructor?

You should never write a non-trivial destructor without writing a copy constructor.

If you explicitly allocate any resources in your constructor, you should write a destructor.

You should possibly think about disabling your copy constructor, as follows:

Ie:
class Foo {  // ...  private:    Foo(const Foo& other); // DO NOT IMPLEMENT THIS    void operator=(const Foo& other); // DO NOT IMPLEMENT THIS};

For most classes, doing the above is a good thing (tm).

[Edited by - NotAYakk on November 15, 2006 9:23:02 PM]
Xrystal
Xrystal
I'm not sure what you mean by trivial, but most of my classes have a blank constructor with one or more constructors that have values passed to it to initialise certain values.

All of my classes have a destructor where I attempt to clean out the class before I'm finished with it.

EG.

DXEngine class has :
DXEngine::DXEngine() and DXEngine::~DXEngine()

whereas

DXFont class has :
DXFont::DXFont() and DXFont::DXFont(LPDIRECT3DDEVICE dev) and DXFont::~DXFont()


As for a copy constructor, I have yet found the need to create a copy of an existing object but I would imagine I would have one set up for that reason. Thinking on things DXEngine shouldn't need a copy constructor but something like DXFont and DXPrimitive classes may find it beneficial to have a copy option. Just haven't thought that far ahead yet :D


Zahlman
Zahlman
Quote:
Original post by Xrystal
I'm not sure what you mean by trivial, but most of my classes have a blank constructor with one or more constructors that have values passed to it to initialise certain values.


"trivial" basically means "not doing anything", or at least not doing anything interesting. A trivial constructor would be understood to be a no-arg one that does everything it has to in the initializer list, generally. A trivial destructor is just '~foo() {}'.

Quote:

As for a copy constructor, I have yet found the need to create a copy of an existing object but I would imagine I would have one set up for that reason.


The language has a habit of copying things implicitly in ways that you might not expect. If you don't want something to be copied, make it explicitly non-copyable, whatever your motives; then the compiler can tell you if it would otherwise be copied against your will.
NotAYakk
NotAYakk
Nearly any class that has a destructor that does anything (beyond simple logging messages) needs to either disable it's copy constructor, or rewrite the copy constructor.

Writing a non-buggy copy constructor is not all that easy. So just disable it.

If you must write one, note that post-copy-constructor all resources shared by both instances of the class need to be able to survive having two destructors called on them. In general, any resources cleaned up in a destructor needs to be duplicated during a copy constructor.

But, really, just disable your copy constructors on any class that has a destructor that needs to do anything.

Topic Locked

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

Sign in to reply to this topic.