Is This Thread Safe?
In my quest to understand the structure and functioning of the Helix Producer SDK code, I am looking at it one tiny piece at a time. Since it is our goal to extract useful code from Helix Producer to include in our project, someone needs to understand the dependencies that we might encounter. Today I started examining the system of smart pointers.
Digression - what is a smart pointer ?
Smart pointers, sometimes referred to as reference counted pointers, are a memory management technique used by C++ programmers. Unlike Java, C# or Python, unused memory is not automatically garbage collected in C++. By adding a layer of indirection, an instance of a special pointer class wraps a traditional C style pointer. This class instance has special copy and assignment constructor semantics: when it is copied, a counter on the thing it is pointing to is incremented. When one of these pointer wrappers is destructed, the counter is decremented. If the counter goes to zero, then it is known that the thing pointed to is no longer referenced by anything and can itself be deallocated.
Smart pointers must be very careful in multi-threaded environments. The act of copying itself must be atomic - very atomic. How does a smart pointer copy itself? Using a copy constructor, it first allocates a memory for a new instance of the smart pointer class and then initializes the instance variables. The only instance variable is the pointer to the thing of interest. Once the pointer is copied, then it increments the reference counter in the pointed-to object. What would happen if the original smart pointer’s destructor were called in the instant between the copy of the pointer and incrementing the reference counter? It is possible that the referenced object could be destructed before the copy gets the chance to increment the reference counter.
Helix Producer Smart Pointers Appear to be Vulnerable
Check out this the copy constructor from Helix Producer’s Smart Pointer Code in producersdk/common/include/hxtsmartpointer.h:
00113 template<class T>
00114 inline CHXTSmartPtr<T>::CHXTSmartPtr( const CHXTSmartPtr<T> &spCopy ) :
00115 m_ptr( spCopy.m_ptr )
00116 {
00117 if ( m_ptr )
00118 {
00119 m_ptr->AddRef();
00120 }
00121 }
The actual pointer is copied in the initializer on line 00115. Then, in the body of the copy constructor, if the pointer is not zero, the referenced object’s counter is incremented. The member function “AddRef” is implemented in producersdk/common/include/hxtunknown.h as:
00084 STDMETHOD_(ULONG32, AddRef) (void)
00085 {
00086 return InterlockedIncrement( &m_lCount );
00087 }
A little spelunking reveals that “InterlockedIncrement” is an atomic operation defined in a Microsoft API. This function call is properly protected in a multi-threaded environment.
As I stated above, though, I believe that both the copy of the referenced pointer and the increment must be protected. How is data shared between threads? It doesn’t seem to matter which thread invokes the copy constructor, the code is vulnerable.
Scenario 1: If a reference to a smart pointer is given to a thread and that thread invokes the copy constructor to make its own copy, then it is vulnerable to accessing deallocated memory. The original smart pointer could have been destructed between the time of the internal pointer’s assignment and incrementing the reference counter.
Scenario 2: If a master thread makes its own copy of the smart pointer object for use by a child thread, then the master thread must explicitly take care to not allow the copy go out of scope while the thread still lives. If the master thread lets the smart pointer destructor be invoked, then the thread has an invalid copy. If the child thread were to make its own copy, then we’re right back to scenario 1.
Does This Really Happen in Helix Producer?
I do not know. I will need to study how the threading model works in our target application. Perhaps we’ll be lucky and referenced counted objects are never shared between threads, but I doubt it.
Further problems with CHXTSmartPtr
CHXTSmartPtr’s assignment operator fails to take into account the tautological “self assignment“.
CHXTSmartPtr<SomeCHTXUnknownDerivative> p(new SomeCHTXUnknownDerivative); CHXTSmartPtr<SomeCHTXUnknownDerivative>& q(p); p = q; //deallocated memory referenced
Granted, code such as direct as this would not be written. However, it is not unfathomable for the provenance of the R-value to be unknown. In such a situation, a self assignment could be completely inadvertent but ultimately disasterous.
November 16th, 2007 at 8:00 am
A little spelunking reveals that “InterlockedIncrement” is an atomic operation defined in a Microsoft API. This function call is properly protected in a multi-threaded environment.