Correct way to release a COM interface

Question

We have some legacy software code, which uses COM. I've noticed that at one place, we perform a queryInterface on an IInspectable pointer, but dont bother calling release.

For ex:

void foo(IInspectable* myInterface) { ComPtr<OBJTYPE> pObj; auto hr = myInterface->QueryInterface(__uuidof(SomeType),&pObj); if (hr != S_OK) { return hr; } //some code that uses pObj }

I added Release to the above code like below :

auto hr = myInterface->QueryInterface(__uuidof(SomeType),&pObj); if (hr != S_OK) { return hr; } myInterface->Release(); //<-- Is this the correct way to release ? //some code that uses pObj;

But I see that the above code crashes at times in the above function during release. Is there something wrong in the way I'm releasing the interface ? I can confirm that myInterface is not being used anywhere else in the function after the Release statement.

Apologies for not being able to copy/paste the actual code, but the above code summarizes pretty much the suspect code that I'm debugging. Basically what I want to know is, in the above scenario, do I need to call Release() ? And is this is the right place/way to call Release() ? Do I need to add any other safety checks in place ?

ComPtr seems to be part of the WRL, not C++/WinRT. The ComPtr destructor will call Release for you. Explicitly calling it will cause a double-delete bug. Since you are passing myInterface as a plain pointer, it means that no ownership on the interface is transferred; this is a non-owning pointer. You are only responsible for the OBJTYPE interface, and ComPtr does that for you. — IInspectable
– IInspectable, Commented Sep 25, 2018 at 7:17
Microsoft::WRL::ComPtr is a COM smart-pointer very similar to ATL's CComPtr. You don't need to call Release explicitly because the smart-pointer does that for you. See ComPtr. C++/WinRT has a similar winrt::com_ptr you can use instead of WRL. — Chuck Walbourn
– Chuck Walbourn, Commented Sep 25, 2018 at 7:20

M.M · Accepted Answer · 2018-09-25 07:13:34Z

In the first foo function you should NOT call myInterface->Release(). Every call to Release should be paired with an adding of a reference. In that function you don't do myInterface->AddRef, so if you add myInterface->Release then your function will decrement the reference count by one, which does not seem like correct behaviour.

Conceptually, you should call AddRef on entry to the function and Release on exit of the function. Even though the reference count is "intrusive" in COM, the recommended coding style is to treat each interface pointer like it has its own reference count.

This is normally wrapped up in smart pointer classes. But when working with a function that receives a raw "in" interface pointer, this Add and Release can be "optimized" by just omitting both of those calls in the function, and treating the pointer like an observer pointer.

The call to QueryInterface will implicitly do pObj->AddRef() but this is nothing to do with myInterface. When you are finished with pObj a pObj->Release() should occur -- but this is managed by the ComPtr wrapper, you shouldn't add an explicit call.

"but this is nothing to do with myInterface" is a bit of a stretch. In the vast majority of cases a call to QueryInterface will return a pointer to an interface implemented by the same object as the callee. I believe tear-off interfaces are the only exception. Reference counting is done per object, not per interface.
I'm not objecting to that, just that the final paragraph steps away from conceptual thinking, and makes a statement that is generally wrong. I'm not sure whether that paragraph should be reworded, or removed entirely. It doesn't add much value as written.

Collectives™ on Stack Overflow

Correct way to release a COM interface

1 Answer 1

3 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Related