Jump to content
  • Advertisement
Sign in to follow this  
noodleBowl

Direct X 11: Display Adapter clean up and mem leaks

This topic is 2080 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I am rewriting some code to deal with the clean up of my display adapters. And it is leaving me wondering am I doing this right?

 

I have this Struct for all display adapters that are found and a vector to store them all in:

struct DisplayAdapter
{
	DisplayAdapter(IDXGIAdapter *conAdapter);
	~DisplayAdapter();

	void cleanUp();

	IDXGIAdapter* adapter;
	DXGI_MODE_DESC* resolutions;
	DXGI_ADAPTER_DESC description;
	UINT numberOfResolutions;
};
std::vector<DisplayAdapter> availableDisplayAdapters;

And the Struct clean up method looks like this:

void SystemX11::DisplayAdapter::cleanUp()
{
	if(adapter != NULL)
		adapter->Release();

	delete [] resolutions;

	numberOfResolutions = NULL;
	ZeroMemory(&description, sizeof(DXGI_ADAPTER_DESC));
}

Where my display adapter find method is:

bool SystemX11::findDisplayAdapters()
{
	cleanDisplayAdapters();

	HRESULT hr;

	IDXGIFactory *factory;
	hr = CreateDXGIFactory(__uuidof(IDXGIFactory), (void**)(&factory));

	if(FAILED(hr))
	{
		std::cout<<"Failed to create the Factory"<<std::endl;
		return false;
	}

	IDXGIAdapter *tempAdapter;
	for(UINT i = 0; factory->EnumAdapters(i, &tempAdapter) != DXGI_ERROR_NOT_FOUND; ++i)
	{
		availableDisplayAdapters.push_back(DisplayAdapter(tempAdapter));
	}
	factory->Release();

	return true;
}

The first line of code (cleanDisplayAdapters method) called from the above code:

bool SystemX11::cleanDisplayAdapters()
{
	if(availableDisplayAdapters.empty())
		return false;

	for(std::vector<DisplayAdapter>::iterator i = availableDisplayAdapters.begin(); i != availableDisplayAdapters.end(); i++)
	{
		(*i).cleanUp();
	}
	availableDisplayAdapters.clear();

	return true;
}

Is this going to leave me with memory leaks? Am I cleaning up the COM objects and dynamic array's (the resolution array created in the Struct constructor) correctly?

 

Also why is that if I add: tempAdapter->release() in the findDisplayAdapters method at the very end I get a access violation? Is this because of the EnumAdapters method? Does this actually set the tempAdapter variable to DXGI_ERROR_NOT_FOUND after it can't find anymore devices?

Share this post


Link to post
Share on other sites
Advertisement
Also why is that if I add: tempAdapter->release() in the findDisplayAdapters method at the very end I get a access violation?

the interface gets destoryed but "adapter" is not set to null

so the "dangling pointer" gets accessed in cleanUp()

I think,,,

Share this post


Link to post
Share on other sites

It look's okay to me.

But if you want to test it you can get VLD( It's free smile.png.

 

This is awesome! Thanks for showing me this tool

 

 

the interface gets destoryed but "adapter" is not set to null

so the "dangling pointer" gets accessed in cleanUp()

I think,,,

 

I think we are talking about two different methods

 

if I add tempAdapter release at the end of this method it breaks on me. I think this because tempAdapter get set to NULL when its done looping through all of the available adapters

bool SystemX11::findDisplayAdapters()
{
	cleanDisplayAdapters();

	HRESULT hr;

	IDXGIFactory *factory;
	hr = CreateDXGIFactory(__uuidof(IDXGIFactory), (void**)(&factory));

	if(FAILED(hr))
	{
		std::cout<<"Failed to create the Factory"<<std::endl;
		return false;
	}

	IDXGIAdapter *tempAdapter;
	for(UINT i = 0; factory->EnumAdapters(i, &tempAdapter) != DXGI_ERROR_NOT_FOUND; ++i)
	{
		availableDisplayAdapters.push_back(DisplayAdapter(tempAdapter));
	}
	factory->Release();
        tempAdapter->Release(); // <---- Breaks if this is here like so

	return true;
}

Either way, you bring up a good point about the hanging pointers, so I would just set all my pointers to NULL when I'm done with them right? Like:

IDXGIAdapter *tempAdapter;

/*Do Temp Adpter Stuff*/

if(tempAdapter != NULL)
      tempAdapter->Release();
tempAdapter = NULL;

Share this post


Link to post
Share on other sites

	IDXGIAdapter *tempAdapter;
	for(UINT i = 0; factory->EnumAdapters(i, &tempAdapter) != DXGI_ERROR_NOT_FOUND; ++i)
	{
		availableDisplayAdapters.push_back(DisplayAdapter(tempAdapter));
                tempAdapter->Release();  // FIXED.
	}
	factory->Release();


 
 
L. Spiro Edited by L. Spiro

Share this post


Link to post
Share on other sites

IDXGIAdapter *tempAdapter;	for(UINT i = 0; factory->EnumAdapters(i, &tempAdapter) != DXGI_ERROR_NOT_FOUND; ++i)	{		availableDisplayAdapters.push_back(DisplayAdapter(tempAdapter));                tempAdapter->Release();  // FIXED.	}	factory->Release();
L. Spiro

I feel like I missed something here. Would this not just release the COM object, making the pointer thats placed into the vector point to nothing?

Share this post


Link to post
Share on other sites

Only if the DisplayAdapter that he's placing in the vector doesn't increase IDXGIAdapter's reference count.

Edited by Mona2000

Share this post


Link to post
Share on other sites
Hey noodleBowl, you're right that releasing the tempAdapter inside your loop like L. Spiro suggests will make each DisplayAdapter.adapter member point to already released IDXGIAdapter COM objects, unless you also addref them in your DisplayAdapter constructor. I'm assuming you don't addref in the constructor, and the cleanup() member function is solely responsible for releasing them.

Going back to your previous post, you shouldn't need to release the tempAdapter outside of the loop either. The loop exits when EnumAdapters returns DXGI_ERROR_NOT_FOUND, which is to say when there is no valid display adapter for the given adapter index.

If there is no valid adapter at that index, depending on the behaviour of EnumAdapters (I don't know exactly, you can use a watch/breakpoint to find out though), then when the loop exits tempAdapter will either be assigned NULL by EnumAdapters, or still point to the IDXGIAdapter COM object enumerated by the last successful loop. If it's NULL you can't release it because it doesn't point anywhere, and if it points to the last successful loop's IDXGIAdapter you don't want to release it because that's the responsibility of the cleanup() function of the last DisplayAdapter in your vector. So either way, no need to release tempAdapter. smile.png Edited by backstep

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!