I need help with a memory leak that appears to be pointing at an empty constructor. ?

Started by
11 comments, last by Matias Goldberg 13 years, 8 months ago
Hi,

I am running into a memory leak that I am detecting through Visual Studio Debug Heap functions...

Hear is the call stack..
msvcr100d.dll!_heap_alloc_dbg_impl(unsigned int nSize, int nBlockUse, const char * szFileName, int nLine, int * errno_tmp)  Line 393	C++msvcr100d.dll!_nh_malloc_dbg_impl(unsigned int nSize, int nhFlag, int nBlockUse, const char * szFileName, int nLine, int * errno_tmp)  Line 239 + 0x19 bytes	C++msvcr100d.dll!_nh_malloc_dbg(unsigned int nSize, int nhFlag, int nBlockUse, const char * szFileName, int nLine)  Line 302 + 0x1d bytes	C++msvcr100d.dll!malloc(unsigned int nSize)  Line 56 + 0x15 bytes	C++msvcr100d.dll!operator new(unsigned int size)  Line 59 + 0x9 bytes	C++DirectX Demo.exe!std::_Allocate<std::_Container_proxy>(unsigned int _Count, std::_Container_proxy * __formal)  Line 36 + 0x15 bytes	C++DirectX Demo.exe!std::allocator<std::_Container_proxy>::allocate(unsigned int _Count)  Line 187 + 0xb bytes	C++DirectX Demo.exe!std::_String_val<wchar_t,std::allocator<wchar_t> >::_String_val<wchar_t,std::allocator<wchar_t> >(std::allocator<wchar_t> _Al)  Line 469 + 0xa bytes	C++DirectX Demo.exe!std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >()  Line 550 + 0x58 bytes	C++DirectX Demo.exe!Sound::Sound()  Line 9	C++DirectX Demo.exe!Audio::LoadSound(std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > path, std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > ID, bool loop)  Line 64 + 0xb bytes	C++DirectX Demo.exe!MainMenuState::Init()  Line 182	C++DirectX Demo.exe!StateManager::AddState(State * state)  Line 13 + 0x23 bytes	C++DirectX Demo.exe!Game::InitializeApp()  Line 43	C++DirectX Demo.exe!Application::InitializeEngine(HINSTANCE__ * instance)  Line 71 + 0xf bytes	C++DirectX Demo.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nShowCmd)  Line 106 + 0xf bytes	C++DirectX Demo.exe!__tmainCRTStartup()  Line 547 + 0x2c bytes	CDirectX Demo.exe!WinMainCRTStartup()  Line 371	Ckernel32.dll!768a3677() 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	ntdll.dll!77139d42() 	ntdll.dll!77139d15() 	

When I step back through the stack to the first sign of code that is mine, it is pointing to an empty constructor...

DirectX Demo.exe!Sound::Sound() Line 9 C++
Sound::Sound(void){}

One more step back leads me to where the class object is being created.

> DirectX Demo.exe!Audio::LoadSound(std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > path,
std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > ID, bool loop) Line 64 + 0xb bytes C++
//-------------------------------------------------------------------------------// Perform per-frame update of audio//-------------------------------------------------------------------------------HRESULT Audio::LoadSound( std::wstring path, std::wstring ID, bool loop ){	HRESULT hr = S_OK;	Wave wav;	// Create and clear a sound	Sound sound;	//ZeroMemory( &sound, sizeof( Sound ) );***** BAD NEWS ******	// Use Wave object to open a .wav file	// This will fill out a WAVEFORMATEXTENSIBLE and XAUDIO2_BUFFER structure	if( FAILED( hr = wav.Open( path, sound ) ) )		return hr;	// Store our sound	m_sounds[ID] = sound;	return S_OK;}

m_sounds is of the type

std::map< std::wstring, Sound > m_sounds

Basically I am really confused as to what I am not getting rid of and was looking to see if someone might be able to shed some light on this on.

REgards

Chad

[Edited by - chadsxe on August 21, 2010 9:22:32 AM]
Advertisement
Are any of the classes mentioned higher in call chain singletons or statics?
Quote:Original post by Antheus
Are any of the classes mentioned higher in call chain singletons or statics?


No, I do not use singlteons or a static class at all.

Regards

Chad
Put an empty Sound::~Sound destructor, like this:

Sound::~Sound(){}


Basically, because of the lack of a destructor your STL's m_sounds variable doesn't get the chance to free the memory it uses.

Cheers
Dark Sylinc
Quote:Original post by Matias Goldberg
Put an empty Sound::~Sound destructor, like this:

*** Source Snippet Removed ***

Basically, because of the lack of a destructor your STL's m_sounds variable doesn't get the chance to free the memory it uses.
I may just be misunderstanding, but could you clarify this? The absence of an explicit destructor doesn't mean that there is no destructor, nor that associated resources will not be freed (at least those that are able to clean up after themselves, that is). Unless I'm mistaken, there's really no reason to add an empty, non-virtual destructor to a class, as the compiler-generated version should be equivalent (and sufficient).

But maybe you meant something other than that...
Quote:Original post by chadsxe
When I step back through the stack to the first sign of code that is mine, it is pointing to an empty constructor...
Remember that in a constructor, any member of the class that itself has a default constructor, will get that default constructor called, unless your initialisation list says to do otherwise. In this case the callstack shows a string member being constructed. For that string to not be destructed, the Sound object that itself has this string member is not being destructed. That will itself probably also show up as a leak. Fix that one and you fix this one also.

The unusual thing about this code is that you actually have an empty constructor. If you don't have any extra work to do from the constructor then just don't write a constructor. The compiler can probably do a better job of creating the default constructor on its own, and less code means less potential for bugs.

Matias Goldberg's advice here is unfortunately completely backwards. If you have any empty and non-virtual destructors, then you should delete them. These again are exactly what the compiler will generate for you if you don't write it, and the compiler is more likely to generate a more efficient and inlined destructor. As always, you only need to explicitly free exactly what you explicitly allocate.

Lastly, don't use (void), that's an old C throwback that is totally unnecessary. Just write ().
"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms
Hi,

Thanks to all for the information so far, but I have done what has been suggested and I am still getting the same memory leak. What do you suggest now? I am still mightly confused.

Regards

Chad
Well,

I think I just figured out my issue. I was making a call like so

ZeroMemory( &sound, sizeof( Sound ) );

in my LoadSound function. That turned out to be bad news. I made the changes to the orginal post to reflect this, and am now going to do some testing to insure this is correct.

Thanks for the info so far. I will report back with the results.

Chad
The exact leak being shown looks to be impossible unless you're doing something nasty/funky with a custom copy constructor for Sound such as a blind memcpy or similar -- and even then, the exact symptoms don't match up with what I'd expect.

My best guess is that it's misreporting and that the real leak is because Audio (and by extension the Sounds it contains in m_sounds) isn't or hasn't been destroyed when you call _CrtDumpMemoryLeaks. Assuming you create Application inside of WinMain, fixing the apparent leak may be as simple as restructing it to be something like:

int WinMain(HINSTANCE * hInstance, HINSTANCE * hPrevInstance, char * lpCmdLine, int nShowCmd) {    // ...    {        Application app;        app.InitializeEngine(...);    }    _CrtDumpMemoryLeaks(...);    return 0;}

The extra curley braces means app goes out of scope and gets destroyed before you dump the memory leaks. If this doesn't help / doesn't seem to be it, I'd recommend posting the entire source for Sound and Wave.
Quote:Original post by chadsxe
Well,

I think I just figured out my issue. I was making a call like so

ZeroMemory( &sound, sizeof( Sound ) );

That wasn't commented out?

Yeah, ZeroMemory is extremely bad mojo. It will trample any memory that's been initialized to anything non-zero in any constructor, so could very easily be it.

This topic is closed to new replies.

Advertisement