CODEWITHSUNDEEP
c++memory-leaksvisual-leak-detector
Ryo
Ryo

Reputation: 1035

Memory Leak with CComPtr

I using crtdbg to detect leak position and I got memory leak when calling new

CComPtr<IDBColumnInfo> m_spColumnInfo
CComPtr<CDBColumnInfo> spResult = new CDBColumnInfo(); //Memory leak here
//another logic come here to set data to spResult
//another logic come here to set data to spResult
//another logic come here to set data to spResult
m_spColumnInfo = static_cast<IDBColumnInfo*>(spResult.Detach());
spResult.Release();

Are there any step need to do with spResult?

Upvotes: 0

Views: 542

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 595295

You have a memory leak, because you are mismanaging the CDBColumnInfo object's reference count.

When you initialize spResult, the object's refcount is initialized to 1. When you call spResult.Detach(), the object still has a refcount of 1, as Detach() does not decrement it. When the detached pointer is then assigned to m_spColumnInfo, the object's refcount is incremented to 2. When m_spColumnInfo is released later on, it decrements the object's recount to 1, and the object is leaked.

You should not be detaching spResult at all. Assign it as-is to m_spColumnInfo, which will increment the refcount to 2, and then let spResult go out of scope normally, decrementing the refcount to 1, leaving m_spColumnInfo with the only active reference. When m_spColumnInfo is then released later on, the refcount will be decremented to 0, and the object will be freed.

You shouldn't be trying to manage the refcount manually at all. That defeats the whole purpose of using CComPtr. 

CComPtr<IDBColumnInfo> m_spColumnInfo;

...

{
    CComPtr<CDBColumnInfo> spResult = new CDBColumnInfo();
    //set data to spResult
    m_spColumnInfo = spResult;
}

Also, on a side note, your function has no business calling CoInitialize() and CoUninitialize() at all! You need to remove those calls from your function (especially since your function does not even call CoUninitialize() in most of the code paths that exit your function). Those calls are not your function's responsibility to make. It is the responsibility of the thread that calls your function to decide how it needs to initialize COM for itself. Those COM functions should be called only once per thread, not per user function.

Upvotes: 1

Related Questions