Gotcha: CComPtrBase assignment

Today what started out as a crash due to a pure virtual call turned into finding a gotcha in CComPtrBase<T>.  Essentially the code in question boiled down to the following.  Can you spot the problem?

 void GetAStudent(CComPtrBase<T> &spStudent)
{
    CComPtr<Student> spLocal;
    // Do some operation to get a student
    spLocal = spStudent;
}

The problem isn't apparent until you look at the definition for CComPtrBase<T>::operater =.  See the problem?  Basically CComPtrBase<T>::operator= isn't explicitly defined.  This means that C++ will automatically implement memberwise assignment.  The RHS of the operator= will be a "const CComPtrBase<T>&". 

CComPtr<T> derives from CComPtrBase<T> therefore it satisfies this and a memberwise assignment will occur.  We now have two smart pointers on the same value.  However the second smart pointer, CComPtrBase<T>, did not perform an AddRef.  So when both objects are destroyed there will be an extra Release and hopefully a crash.

The fix?

1. Use CComPtr<T> or CComPtrEx<T> instead of CComPtrBase<T>
2. Less Optimal: call AddRef() on spLocal.

Comments

• Anonymous
  April 08, 2008

3. Use dumb pointers as function parameters: void GetAStudent(T *pStudent)

• Anonymous
  April 08, 2008
  For a usage case I agree that raw pointers may be an easier solution.  However this is resource aquisition case.  The function is retrieving and returning an instance of Student.  Using a raw pointer provides three ways to fail

1. Caller case pass NULL
2. Caller can use a non-RAII friendly pointer type leading to a possible memory leak
3. Caller may not understand that this is a COM object vs a standard heap object.  Using an std::auto_ptr would compile without errors but would cause a crash (hopefully) Using a reference parameter of an RAII friendly type prevents all of these bugs from occurring.  Yes it should be the responsibility of the programmer to investigate your API usage.  But isn't it better to prevent the mistake outright?