views:

91

answers:

3

I have a COM interface with a method that returns an object:

interface ICreatorInterface {
    HRESULT CreateObject( IObjectToCreate** );
};

The key is that calling ICreatorInterface::CreateObject() is the only way to retrieve an object implementing IObjectToCreate interface.

In C++ I could do it this way:

 HRESULT CCreatorInterfaceImpl::CreateObject( IObjectToCreate** result )
 {
     //CObjectToCreateImpl constructor sets reference count to 0
     CObjectToCreateImpl* newObject = new CObjectToCreateImpl();
     HRESULT hr = newObject->QueryInterface( __uuidof(IObjectToCreate), (void**)result );
     if( FAILED(hr) ) {
         delete newObject;
     }
     return hr;
 }

or this way

 HRESULT CCreatorInterfaceImpl::CreateObject( IObjectToCreate** result )
 {
     //CObjectToCreateImpl constructor sets reference count to 1
     CObjectToCreateImpl* newObject = new CObjectToCreateImpl();
     HRESULT hr = newObject->QueryInterface( __uuidof(IObjectToCreate), (void**)result );
     // if QI() failed reference count is still 1 so this will delete the object
     newObject->Release();
     return hr;
 }

The difference is how the reference counter is initialized and how the object deletion is implemented in case QueryInterface() fails. Since I fully control both CCreatorInterfaceImpl and CObjectToCreateImpl I can go either of ways.

IMO the first variant is clearer - all reference-counting stuff is in one piece of code. Have I overseen something? Why could the second approach be better? Which of the above is better and why?

A: 

Both variations violate a very fundamental principle of COM

  • Never call any method, other than AddRef, on a COM object that has a ref count of zero.

To do otherwise leads to all sorts of errors. Simply put because it prevents people from doing completely legal operations on the object. Like putting them into a smart pointer. The smart pointer would call AddRef, put the count to 1, and later Release putting the count to 0 and causing the object to self destruct.

Yes I realize that 90% of the implementations of QueryInterface don't do this. But I also guarantee you that there are some out there that do :)

I think the simplest approach is to call AddRef immediately after creating the object. This allows the object to behave like a normal COM object at the earliest possible moment.

I've run into this problem in the past and I've written a nice little helper method (assuming the object is implemented in ATL).

template <class T>
static 
HRESULT CreateWithRef(T** ppObject)
{
    CComObject<T> *pObject;
    HRESULT hr = CComObject<T>::CreateInstance(&pObject);
    if ( SUCCEEDED(hr) )
    {
        pObject->AddRef();
        *ppObject = pObject;
    }

    return hr; 
}
JaredPar
I see your point, but the very same behaviour can be achieved much simpler: init the refcount with 0 in constructor and use a smart pointer in the code in question - just bind the new'ed object to the smart pointer, then call QI() - ATL::CComPtr allows that, perhaps _com_ptr_t allows too.
sharptooth
+2  A: 

Raymond Chen wrote a relevant article on his weblog: On objects with a reference count of zero

jamesdlin
A: 

I always use the following code scenario for creating returned com objects to avoid issues with memory. Of course this works because my objects are reference counted = 0 when created. This always just seems clearer to me than trying to handle the error condition with using the delete operator.

 HRESULT CCreatorInterfaceImpl::CreateObject( IObjectToCreate** result )
 {
     //CObjectToCreateImpl constructor sets reference count to 0
     CObjectToCreateImpl* newObject = new CObjectToCreateImpl();

     newObject->AddRef();

     HRESULT hr = newObject->QueryInterface( __uuidof(IObjectToCreate), (void**)result);

     newObject->Release(); // release my addref, if QI succeeded it AddRef'd, if not the object is destroyed

     return hr; // return result from QI
 }
Ruddy
The same effect could be achieved with two times less code if you used a smart pointer like CComPtr.
sharptooth
True, only minor problem would be if the code needed to call some non-interface based method on the object (Such as an internal Initialize - that wouldn't have been handled in the constructor due to potential error situations - or what have you) you'd still need a pointer to the proper type rather than the interface.
Ruddy
In most cases you can parameterize CComPtr with the actual most-derived type, not the interface.
sharptooth