Odd Memory Leak Problem - Objects Not Being Released

Started by
11 comments, last by Mezz 19 years, 3 months ago
I have a huge memory leak in my application, and I can't figure it out. I've been fiddling around with my CleanUp() function, and discovered something odd. For starters, here's the clean up function, located in Direct3D.cpp:

void CDirect3D::CleanUp()
{
	if( m_pPlayerTile )
		m_pPlayerTile->Release();
	m_pPlayerTile = 0;
	
	if( m_pCurrentTileSet )
		m_pCurrentTileSet->Release();
	m_pCurrentTileSet = 0;
	
	if( m_pBackBuffer )
		m_pBackBuffer->Release();
	m_pBackBuffer = 0;
		
	if( m_pD3DDevice )
		m_pD3DDevice->Release();
	m_pD3DDevice = 0;
	
	if( m_pD3D )
		m_pD3D->Release();
	m_pD3D = 0;
}






I commented out every section one by one, and saw how much it leaked. Upon not cleaning out my player tile, the bytes leaked increased. Same for the m_pCurrentTileSet. However, the number of bytes leaked did not change when I commented out the code to release the back buffer, device and d3d object. So, effectively, my function was the equivalent of:

void CDirect3D::CleanUp()
{
	if( m_pPlayerTile )
		m_pPlayerTile->Release();
	m_pPlayerTile = 0;
	
	if( m_pCurrentTileSet )
		m_pCurrentTileSet->Release();
	m_pCurrentTileSet = 0;
}






Why are the objects not being released, even when I explicitely call the Release methods and set them all to 0? I have no idea what the problem is, so I don't know what to show you. Here's the whole .cpp:

// Direct3D.cpp: implementation of the CDirect3D class.
//
//////////////////////////////////////////////////////////////////////

#include "Direct3D.h"

//////////////////////////////////////////////////////////////////////
// Construction/Destruction
//////////////////////////////////////////////////////////////////////

CDirect3D::CDirect3D()
{
	m_pD3D = NULL;
	m_pD3DDevice = NULL;
	m_pBackBuffer = NULL;
	m_pCurrentTileSet = NULL;
	m_pPlayerTile = NULL;
	strcpy( m_szErrorMsg, "No Error" );
}

CDirect3D::~CDirect3D()
{
	CleanUp();




	/////////////////////
	//_CrtDumpMemoryLeaks();
	///////////////////////






}

CDirect3D::CDirect3D( CVisual& vis )
{
	VISUAL = vis;
}

////////////////////////////////
// SetWindowHandle()
////////////////////////////////
void CDirect3D::SetWindowHandle( HWND hWnd )
{
	m_hWnd = hWnd;
}

////////////////////////////////
// InitD3D()
////////////////////////////////
HRESULT CDirect3D::InitD3D()
{
	HRESULT hResult = CreateD3DObject();
	if( hResult != D3D_OK )
		return hResult;
	hResult = CheckDisplayMode();
	if( hResult != D3D_OK )
		return hResult;
	hResult = CreateD3DDevice();
	if( hResult != D3D_OK )
		return hResult;
	hResult = CreateSurfaces();
	return hResult;
}

////////////////////////////////
// CreateD3DObject()
////////////////////////////////
HRESULT CDirect3D::CreateD3DObject()
{
	m_pD3D = Direct3DCreate8( D3D_SDK_VERSION );
	if( m_pD3D == NULL )
	{
		MessageBox( m_hWnd,
		"Couldn't create DirectX object.",
		"DirectX Error",
		MB_OK | MB_ICONHAND );
		strcpy( m_szErrorMsg, "CreateD3DObject()" );
		return E_FAIL;
	}
	return D3D_OK;
}

////////////////////////////////
// CheckDisplayMode()
////////////////////////////////
HRESULT CDirect3D::CheckDisplayMode()
{
	HRESULT hResult = m_pD3D->CheckDeviceType( D3DADAPTER_DEFAULT,
		D3DDEVTYPE_HAL, D3DFMT_X8R8G8B8, D3DFMT_X8R8G8B8, FALSE );
	if( hResult != D3D_OK )
	{
		MessageBox( m_hWnd,
			"The required display mode is not available on this system.",
			"DirectX Error",
			MB_OK | MB_ICONHAND );
		strcpy( m_szErrorMsg, "CheckDisplayMode()" );
		return hResult;
	}
	return D3D_OK;
}

////////////////////////////////
// CreateD3DDevice()
////////////////////////////////
HRESULT CDirect3D::CreateD3DDevice()
{
	D3DPRESENT_PARAMETERS D3DPresentParams;
	ZeroMemory( &D3DPresentParams, sizeof( D3DPRESENT_PARAMETERS ) );
	D3DPresentParams.Windowed = FALSE;
	D3DPresentParams.BackBufferCount = 1;
	D3DPresentParams.BackBufferWidth = 800;
	D3DPresentParams.BackBufferHeight = 600;
	D3DPresentParams.BackBufferFormat = D3DFMT_X8R8G8B8;
	D3DPresentParams.SwapEffect = D3DSWAPEFFECT_DISCARD;
	D3DPresentParams.hDeviceWindow = m_hWnd;
	HRESULT hResult = E_FAIL;
	hResult = m_pD3D->CreateDevice( D3DADAPTER_DEFAULT,
		D3DDEVTYPE_HAL, m_hWnd, D3DCREATE_SOFTWARE_VERTEXPROCESSING,
		&D3DPresentParams, &m_pD3DDevice );
	if( FAILED( hResult ) )
	{
		MessageBox( m_hWnd,
			"Failed to create Direct3D device.",
			"DirectX Error",
			MB_OK | MB_ICONHAND );
		strcpy( m_szErrorMsg, "CreateD3DDevoce()" );
	}
	return hResult;
}

////////////////////////////////
// CreateSurfaces()
////////////////////////////////
HRESULT CDirect3D::CreateSurfaces()
{
	//LoadTileSet( "Graphics\\Tiles\\Default.bmp" );
	// create player tile
	return D3D_OK;
}

////////////////////////////////
// CleanUp()
////////////////////////////////
void CDirect3D::CleanUp()
{
	if( m_pPlayerTile )
		m_pPlayerTile->Release();
	m_pPlayerTile = 0;
	
	if( m_pCurrentTileSet )
		m_pCurrentTileSet->Release();
	m_pCurrentTileSet = 0;
	
	if( m_pBackBuffer )
		m_pBackBuffer->Release();
	m_pBackBuffer = 0;
		
	if( m_pD3DDevice )
		m_pD3DDevice->Release();
	m_pD3DDevice = 0;
	
	if( m_pD3D )
		m_pD3D->Release();
	m_pD3D = 0;
}

////////////////////////////////
// GetBackBuffer()
////////////////////////////////
IDirect3DSurface8* CDirect3D::GetBackBuffer()
{
	return m_pBackBuffer;
}

////////////////////////////////
// GetDevice()
////////////////////////////////
LPDIRECT3DDEVICE8 CDirect3D::GetDevice()
{
	return m_pD3DDevice;
}

////////////////////////////////
// GetErrorString()
////////////////////////////////
char* CDirect3D::GetErrorString()
{
	return m_szErrorMsg;
}

////////////////////////////////
// LoadTileSet()
////////////////////////////////
BOOL CDirect3D::LoadTileSet( const char* filename )
{
	if( m_pCurrentTileSet )
		m_pCurrentTileSet->Release();

	HRESULT hResult;		// To check success
	
	hResult = m_pD3DDevice->CreateImageSurface( 256, 256,
		D3DFMT_X8R8G8B8, &m_pCurrentTileSet );
	if( FAILED( hResult ) )
	{
		CloseWindow( m_hWnd );
		MessageBox( m_hWnd,
			"Failed to create the tileset surface.\nPlease contact 
the program vendor about this error.",
			"Error",
			MB_OK | MB_ICONHAND );
		PostQuitMessage( FailCreateTilesetSurface );
		return FALSE;
	}

	hResult = D3DXLoadSurfaceFromFile( m_pCurrentTileSet,
		NULL, NULL, filename, NULL, D3DX_FILTER_NONE, 0, NULL );
	if( FAILED( hResult ) )
	{
		char ErrorMessage[ 1024 ];
		wsprintf( ErrorMessage, "Failed to load tileset %s.\nPlease make sure it exists 
and that is is a valid image tileset.", filename );
		CloseWindow( m_hWnd );
		MessageBox( m_hWnd,
			ErrorMessage,
			"Error",
			MB_OK | MB_ICONHAND );
		return FALSE;
	}

	return TRUE;
}

////////////////////////////////
// GetPlayerTile()
////////////////////////////////
IDirect3DSurface8* CDirect3D::GetPlayerTile( void )
{
	return m_pPlayerTile;
}

////////////////////////////////
// CreatePlayerTile()
////////////////////////////////
BOOL CDirect3D::CreatePlayerTile( const char* filename, CVisual& Visual_UpdatePlayerElements )
{
	HRESULT hResult;

	D3DXIMAGE_INFO Info;
	D3DXGetImageInfoFromFile( filename, &Info );
	int width = static_cast<int>(Info.Width);
	int height = static_cast<int>(Info.Height);
	if( width != height )
	{
		char ErrorBuffer[ 1024 ];
		wsprintf( ErrorBuffer, "Player tile %s \nis not a valid tile 
- width does not equal height.", filename );
		CloseWindow( m_hWnd );
		MessageBox( m_hWnd, ErrorBuffer, "Error", MB_OK | MB_ICONHAND );
		PostQuitMessage( FailCreatePlayerSurface );
		return FALSE;
	}
	if( width > Visual_UpdatePlayerElements.TILESIZE || height > 
Visual_UpdatePlayerElements.TILESIZE )
	{
		char ErrorBuffer[ 1024 ];
		wsprintf( ErrorBuffer, "Player tile %s \nexceeds the specified 
tile size of %d.", filename, Visual_UpdatePlayerElements.TILESIZE );
		CloseWindow( m_hWnd );
		MessageBox( m_hWnd, ErrorBuffer, "Error", MB_OK | MB_ICONHAND );
		PostQuitMessage( FailCreatePlayerSurface );
		return FALSE;
	}
	if( width % 2 || height % 2 )
	{
		char ErrorBuffer[ 1024 ];
		wsprintf( ErrorBuffer, "Error in player tile %s:\nImage dimensions must be even numbers.", filename );
		CloseWindow( m_hWnd );
		MessageBox( m_hWnd, ErrorBuffer, "Error", MB_OK | MB_ICONHAND );
		PostQuitMessage( FailCreatePlayerSurface );
		return FALSE;
	}
	Visual_UpdatePlayerElements.PLAYERTILESIZE = width;

	hResult = m_pD3DDevice->CreateImageSurface( 
Visual_UpdatePlayerElements.PLAYERTILESIZE, 
Visual_UpdatePlayerElements.PLAYERTILESIZE,
		D3DFMT_X8R8G8B8, &m_pPlayerTile );
	if( FAILED( hResult ) )
	{
		CloseWindow( m_hWnd );
		MessageBox( m_hWnd,
			"Failed to create player surface.\nPlease contact 
the program vendor about this problem.",
			"Error",
			MB_OK | MB_ICONHAND );
		PostQuitMessage( FailCreatePlayerSurface );
		return FALSE;
	}

	hResult = D3DXLoadSurfaceFromFile( m_pPlayerTile,
		NULL, NULL, filename,
		NULL, D3DX_FILTER_NONE, 0, NULL );
	if( FAILED( hResult ) )
	{
		char ErrorMessage[ 1024 ];
		wsprintf( ErrorMessage, "Cannot find the default player tile. 
Please make sure this player tile exists:\n%s", filename );
		CloseWindow( m_hWnd );
		MessageBox( m_hWnd,
			ErrorMessage,
			"Error",
			MB_OK | MB_ICONHAND );
		PostQuitMessage( FailFindDefPlayerSurface );
		return FALSE;
	}
	return TRUE;

	
}

////////////////////////////////
// GetCurrentTileSurface()
////////////////////////////////
IDirect3DSurface8* CDirect3D::GetCurrentTileSurface( void )
{
	return m_pCurrentTileSet;
}

////////////////////////////////
// UpdateVisual()
////////////////////////////////
void CDirect3D::UpdateVisual( const CVisual& vis )
{
	VISUAL = vis;
}

////////////////////////////////
// ClearScreen()
////////////////////////////////
void CDirect3D::ClearScreen( void )
{
	m_pD3DDevice->GetBackBuffer( 0, D3DBACKBUFFER_TYPE_MONO,
		&m_pBackBuffer );
	m_pD3DDevice->Clear( 0, 0, D3DCLEAR_TARGET,
		D3DCOLOR_XRGB( 0, 0, 0 ), 0, 0 );
	m_pD3DDevice->Present( NULL, NULL, NULL, NULL );
	return;
}






And here's the Direct3d.h:

////////////////////////////////
// Direct3D.h
////////////////////////////////

#pragma once

#include <d3d8.h>
#include <d3dx8tex.h>
#include <d3d8.h>
#include "visual.h"
#include "ConstExitCodes.h"

class CDirect3D
{
	protected:
		// Object and device
		LPDIRECT3D8 m_pD3D;
		LPDIRECT3DDEVICE8 m_pD3DDevice;

		// Surfaces
		IDirect3DSurface8* m_pPlayerTile;
		IDirect3DSurface8* m_pCurrentTileSet;

		// Back buffer
		IDirect3DSurface8* m_pBackBuffer;

		// Window handle
		HWND m_hWnd;

		// Error message buffer
		char m_szErrorMsg[ 256 ];
		
	public:
		CDirect3D( void );
		CDirect3D( CVisual& );
		~CDirect3D( void );
		void SetWindowHandle( HWND hWnd );
		HRESULT InitD3D( void );
		LPDIRECT3DDEVICE8 GetDevice( void );
		IDirect3DSurface8* GetBackBuffer( void );
		char* GetErrorString( void );
		BOOL LoadTileSet( const char* );
		IDirect3DSurface8* GetPlayerTile( void );
		BOOL CreatePlayerTile( const char*, CVisual& );
		IDirect3DSurface8* GetCurrentTileSurface( void );
		void UpdateVisual( const CVisual& );
		void ClearScreen( void );
		CVisual VISUAL;

	private:
		HRESULT CreateD3DObject( void );
		HRESULT CheckDisplayMode( void );
		HRESULT CreateD3DDevice( void );
		HRESULT CreateSurfaces( void );
		void CleanUp( void );
};






I'm sorry to dump out all this code, but I really have no idea what the problem could be. EDIT: Also, it would be worth mentioning that when you break on memory allocation number 1, it gives you the disassembly window. Continuing running the program from there (by pressing F10, running it line by line in disassembly), you eventually get to a line of source code, which, I guess, is disassembly window's way of telling you where you are in the actual source file. Using that, I found out I was located precicely where the D3D object of created in the source file. No big surprise there, but it didn't help me determine why the leak happened. In other words, I know what the leak is, but I don't know why. [Edited by - v0dKA on January 1, 2005 11:00:09 AM]
.:<<-v0d[KA]->>:.
Advertisement
Quote:
However, the number of bytes leaked did not change when I commented out the code to release the back buffer, device and d3d object.


OK I see a potential problem. In the majority of cases, every time you call a Get*() method that returns an interface it will increment the reference count of that interface. The biggest example I can see in your code is this:

void CDirect3D::ClearScreen( void ){   m_pD3DDevice->GetBackBuffer( 0, D3DBACKBUFFER_TYPE_MONO, &m_pBackBuffer );   m_pD3DDevice->Clear( 0, 0, D3DCLEAR_TARGET, D3DCOLOR_XRGB( 0, 0, 0 ), 0, 0 );   m_pD3DDevice->Present( NULL, NULL, NULL, NULL );   return;}


Every time you get the back buffer, it's reference count goes up because you don't release it, so you end up with a highly referenced back buffer, which would require many Release() calls before it is actually properly released. An alternative way to code this function might be:

void CDirect3D::ClearScreen( void ){   if(m_pBackBuffer)   {      m_pBackBuffer->Release();      m_pBackBuffer = NULL;   }   m_pD3DDevice->GetBackBuffer( 0, D3DBACKBUFFER_TYPE_MONO, &m_pBackBuffer );   m_pD3DDevice->Clear( 0, 0, D3DCLEAR_TARGET, D3DCOLOR_XRGB( 0, 0, 0 ), 0, 0 );   m_pD3DDevice->Present( NULL, NULL, NULL, NULL );   return;}


Which releases the back buffer so it's reference count doesn't sky rocket. Check your code for cases in which the other objects may have this happen to them.

Hope that helped.

EDIT: infact, I believe it may just be the back buffer which is screwing things up, because I think the D3D device & main interface object won't allow themselves to be released until all their children are released, so if you fix the back buffer problem those objects shouldn't leak memory either.


-Mezz
This unfortunately didn't solve the problem. I added the code you gave, and searched for other such cases.

If you're on the right track, there may be a few potential problems, but I'm not sure this is exactly the same thing. I do use Direct3D objects as arguments and return types commonly, in cases such as (function prototypes given):

IDirect3DSurface8* GetBackBuffer( void );
HRESULT PlaceTile( IDirect3DSurface8* pBackBuffer, ... )

Are these cases a problem? Does the back buffer get reproduced when you return it?
Here's my GetBackBuffer():
IDirect3DSurface8* CDirect3D::GetBackBuffer()
{
return m_pBackBuffer;
}
where m_pBackBuffer is of IDirect3DSurface8* variable type, which is a member variable of the direct3d class.
I don't believe the back buffer gets reproduced in such cases, does it?

Also, this code snippet could be a potential problem (brought to it when breaking on memory allocation number 190):
char buf[ 100 ];	char buf2[ 100 ];	wsprintf( buf, "Player Position: x=%d, y=%d", Map.Player.GetPosition()->x, Map.Player.GetPosition()->y );	wsprintf( buf2, "LowVisibleCol = %d, HiVisibleCol = %d, LowVisibleRow = %d, HiVisibleRow = %d", LowVisibleCol, HiVisibleCol, LowVisibleRow, HiVisibleRow );	LOGFONT lf;	ZeroMemory( &lf, sizeof( LOGFONT ) );	lf.lfHeight = 12;	lf.lfWidth = 7;	lf.lfItalic = false;	lf.lfUnderline = false;	lf.lfStrikeOut = false;	lf.lfCharSet = DEFAULT_CHARSET;	strcpy( lf.lfFaceName, "Times New Roman" );	ID3DXFont* font = 0;	D3DXCreateFontIndirect( m_Direct3D.GetDevice(), &lf, &font );	RECT destRect;	destRect.left = 0;	destRect.right = 800;	destRect.top = 0;	destRect.bottom = 600;			m_Direct3D.GetDevice()->BeginScene();	font->DrawText( buf, -1, &destRect,		DT_TOP | DT_RIGHT, 0xffffffff );	destRect.top += 18;	font->DrawText( buf2, -1, &destRect,		DT_TOP | DT_RIGHT, 0xffffffff );	destRect.top += 18;	font->DrawText( buf3, -1, &destRect,		DT_TOP | DT_RIGHT, 0xffffffff );	m_Direct3D.GetDevice()->EndScene();	font->Release();


I release the font object, though I don't even think that's required. Is there anything wrong with that?
.:<<-v0d[KA]->>:.
I think I may have been unclear in my original post.

All DirectX objects are COM objects, which means they are reference counted, it is the supposed duty of functions that give them out to increment the reference count (via a call to AddRef()) and the receiving code to Release() the interface when done with it. Direct3D adheres to this principle, thus, when you call something like IDirect3DDevice8::GetBackBuffer() internally it will call AddRef() on the surface before returning it to you, thus when you are done you need to call Release() to decrement the reference count. The object is not truly released (i.e. memory freed and destroyed) until the reference count is zero! I hope that makes sense.

So it is no problem in your own GetBackBuffer() and PlaceTile() functions because you do not AddRef() the objects.

It's worth mentioning that Release() actually returns a ULONG value - which is the current reference count of the object. You could save this in your CleanUp() function and print it out for debugging purposes (never use the return value from Release() for anything other than diagnostics!) which would tell you if you're on the right track with reference counts or not.

Quote:
I release the font object, though I don't even think that's required. Is there anything wrong with that?


It is correct & required that you release the font object - every time you get an object from D3D, it is almost certain that you will need to Release() it to avoid memory leaks!

As for the code I originally posted, that was just an example... not entirely sure why it wouldn't work, but ideally you don't want to be getting the back buffer every frame anyway... you could just call m_pD3DDevice->GetBackBuffer() once in your initialisation code.

-Mezz
Wow [wow]!!!!!!

All you needed to say is Release returns the reference count!

Now comes the tiresome task of weeding out the 192 references of LPDIRECT3DDEVICE8's [embarrass].

EDIT: Is there a method to get the reference count? I didn't find any by searching. For now, I'll just call AddRef() and immediately Release() to get the reference count, though I hoped for a better way.

[Edited by - v0dKA on January 1, 2005 12:53:16 PM]
.:<<-v0d[KA]->>:.
No I don't know of any neat way of getting the object's reference count, apart from the AddRef() / Release() way you just mentioned. Mostly because it's only for debugging so they don't really want to provide access to it :)

It'd be pretty easy to wrap up in a neat little function though:

ULONG GetReferenceCount(IUnknown *object){   object->AddRef();   return(object->Release());}


-Mezz
So the plot thickens, in a nasty sort of way.

This one line increases the reference count of the device from 4 (have yet to find where that came from) to a whooping 15:

D3DXCreateFontIndirect( m_Direct3D.GetDevice(), &lf, &font );

What could be responsible for this? This comes from the code snippet in an above post. GetDevice() just returns the LPDIRECT3DDEVICE8* from my CDirect3D class - nothing wrong there. Any ideas?
.:<<-v0d[KA]->>:.
Quote:Original post by v0dKA

D3DXCreateFontIndirect( m_Direct3D.GetDevice(), &lf, &font );


Try it with

IDirect3DDevice9* device = m_Direct3D.GetDevice();
D3DXCreateFontIndirect( device , &lf, &font );
device->Release();

i could think of the case where D3DXCreateFontIndirect is a macro something like

D3DXCreateFontIndirect(x, y, z) x->do_something(); x->do_somethingelse(); etc.

which would expand in your case into

m_Direct3D.GetDevice()->do_something();
m_Direct3D.GetDevice()->do_somethingelse();
etc.

in each "m_Direct3D.GetDevice()" the reference count is incremented but nothing releases the device.

note: this is just an idea, i don't know if thats the case, but i think its worth to try it out ;-)

cheers
chris
Quote:Original post by v0dKA
So the plot thickens, in a nasty sort of way.

This one line increases the reference count of the device from 4 (have yet to find where that came from) to a whooping 15:

D3DXCreateFontIndirect( m_Direct3D.GetDevice(), &lf, &font );

What could be responsible for this? This comes from the code snippet in an above post. GetDevice() just returns the LPDIRECT3DDEVICE8* from my CDirect3D class - nothing wrong there. Any ideas?


That's quite odd indeed. As the AP said, the call is a macro (well, sort of), but your m_Direct3D.GetDevice() call doesn't incremement the reference count (does it?) so it shouldn't mess anything up that way, although it wouldn't hurt to try the way the AP suggested.

Also, you could try doing that font creation and then immediately releasing the font, just to check if that brings the reference count back down by 11 - if so then there is no problem and D3D is just internally referencing it a lot of times.

-Mezz
I just tested this theory in my own code.
The following

            ULONG before = GetRefCount(Renderer()->GetDevice());            HRESULT hr = D3DXCreateFontIndirect(Renderer()->GetDevice(), &fontDesc, &font);            ULONG after = GetRefCount(Renderer()->GetDevice());            font->Release();            ULONG afterAfter = GetRefCount(Renderer()->GetDevice());


Gives me 4, 5, then 4 after the font is released.

Although I am using DirectX 9.

-Mezz

This topic is closed to new replies.

Advertisement