Memory leak in this situation with DirectX COM objects?

Question

If I have a class or struct like this:

bool localScopeFunc()
{
   bool result;
   IDXGIFactory* pFactory;
   IDXGIAdapter* pAdapter; 
  
    result = //Do something here with the objects
 if (!result) return false;
  
 result = //Do something here with the objects
 if (!result) return false;
  
 result = //Do something here with the objects
 if (!result) return false;
  // And so on......
  
  
  //___Do cleanup here___//
  pFactory->Release(); pFactory = nullptr;
  pAdapter->Release(); pAdapter = nullptr;
   
  return true; // If all passes
}

If at any point during this function something fails and it returns false, it doesn't do the cleanup at the end, thus not calling ->Release() on any objects. Will this mean a memory leak?

If so, I can't figure out a feasible way to do it, as sometimes I'll have a list of function calls, at each stage initialising something new, and if I had to clean everything up in reverse it would look like this:

int main()
{
 if (!initTime())    {return -1;}
 if (!initD3D())     {shutDownTime(); return -2;}
 if (!initCamera())  {shutDownD3D(); shutDownTime(); return -3;}
 if (!initSound())   {shutDownCamera(); shutDownD3D(); shutDownTime(); return -3;}
 if (!initPhysics()) {shutDownSound(); shutDownD3D(); shutDownTime(); return -4;}
// And so on.
 
 return 0; 
}

score 2 · Accepted Answer · answered Apr 25 '16 at 06:58

2

Yes, it will leak because you are skipping clean-up. COM objects use reference-counting, and the deal is the count has to be accurate for the system to delete the memory at the right time.

The solution here is actually very easy: Use Microsoft::WRL::ComPtr. This smart-pointer takes care of calling Release if needed no matter how you leave the scope.

The other thing to note is that COM objects don't return errors as bools. They are HRESULTs. You should not ignore them, because if the function returns an HRESULT it can fail. You also shouldn't use == S_OK or the like. You should use the FAILED macro, the SUCCEEDED macro, or something like DX::ThrowIfFailed.

#include <wrl/client.h>

using Microsoft::WRL::ComPtr;

bool localScopeFunc()
{
    ComPtr<IDXGIFactory> pFactory;
    ComPtr<IDXGIAdapter> pAdapter; 

    HRESULT result = //Do something here with the objects
    if (FAILED(result)) return false;

    result = //Do something here with the objects
    if (FAILED(result)) return false;

    result = //Do something here with the objects
    if (FAILED(result)) return false;

    // And so on......

    return true; // If all passes
}

For more on using ComPtr, see this.

answered Apr 25 '16 at 06:58

Chuck Walbourn

38,259
2
58
81

Thanks Chuck. I've only been using raw pointers so far, but the ComPtr and unique_pointer seem very handy for this sort of thing. Would the ComPtr share more similarities with the unique_pointer or shared_pointer? Also, I've been trying to find the difference between the two COM object methods .get() and .getAddressOf(). The I can't understand from the MSDN what it's saying as the wording to me sounds equivalent. Should I pass .get() or getAddressOf() as arguments? Thanks. – Zebrafish Apr 25 '16 at 18:02
``ComPtr`` is a reference-counted shared smart-pointer, so logically similar to ``shared_ptr``. See [this](https://github.com/Microsoft/DirectXTK/wiki/ComPtr) for usage hints for ``ComPtr`` where I specifically talk about why you'll see both ``Get`` and ``GetAddressOf`` used with Direct3D parameters. – Chuck Walbourn Apr 25 '16 at 20:41
Thanks, I've read that about 3 times. It's slowly sinking in. The difference is get() is the pointer and getAddressOf() is the address of the pointer, that you can pass to functions which take pointer to pointer arguments. One other thing if you don't mind explaining. Usually if you create a pointer to something, immediately after you might check to see if it's null, which would indicate a failure. So when I go if ( ! ptr ) return false; , would that need to be changed to if ( ! ptr.get() ) return false; ? As a unique ptr isn't a pointer itself, but a wrapper of one. – Zebrafish Apr 25 '16 at 21:34
The smart-pointers have a ``operator bool`` for precisely that pattern. That said, in the case of COM usually you check the ``HRESULT`` of the factory function rather than checking for a null in the returned pointer. – Chuck Walbourn Apr 25 '16 at 23:21

Memory leak in this situation with DirectX COM objects?

1 Answers1